In the future it is likely that our vlan support will expand to
include multiply tagged packets. When this happens, we would
ideally like for it to be consistent with our current tagging.
Currently, if we receive a packet with a partial VLAN tag we will
automatically drop it in the kernel, which is unique among the
protocols we support. The only other reason to drop a packet is
a memory allocation error. For a doubly tagged packet, we will
parse the first tag and indicate that another tag was present but
do not drop if the second tag is incorrect as we do not parse it.
This changes the behavior of the vlan parser to match other protocols
and also deeper tags by indicating the presence of a broken tag with
the 802.1Q EtherType but no vlan information. This shifts the policy
decision to userspace on whether to drop broken tags and allows us to
uniformly add new levels of tag parsing.
Although additional levels of control are provided to userspace, this
maintains the current behavior of dropping packets with a broken
tag when using the NORMAL action because that is the correct behavior
for an 802.1Q-aware switch. The userspace flow parser actually
already had the new behavior so this corrects an inconsistency.
Signed-off-by: Jesse Gross <je...@nicira.com>
---
datapath/actions.c | 2 +-
datapath/datapath.c | 2 +-
datapath/flow.c | 25 ++++++++++++++++++++-----
lib/dpif-netdev.c | 2 +-
lib/odp-util.c | 24 +++++++++++++++++++-----
ofproto/ofproto-dpif.c | 10 +++++++++-
6 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index 4db2563..2202003 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -113,7 +113,7 @@ static int push_vlan(struct sk_buff *skb, const struct
ovs_key_8021q *q_key)
+ ETH_HLEN, VLAN_HLEN, 0));
}
- __vlan_hwaccel_put_tag(skb, ntohs(q_key->q_tci));
+ __vlan_hwaccel_put_tag(skb, ntohs(q_key->q_tci) & ~VLAN_TAG_PRESENT);
return 0;
}
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 87056cf..be9f979 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -568,7 +568,7 @@ static int validate_action_key(const struct nlattr *a,
if (q_key->q_tpid != htons(ETH_P_8021Q))
return -EINVAL;
- if (q_key->q_tci & htons(VLAN_TAG_PRESENT))
+ if (!(q_key->q_tci & htons(VLAN_TAG_PRESENT)))
return -EINVAL;
break;
diff --git a/datapath/flow.c b/datapath/flow.c
index 9e0b842..f9e8c7b 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -480,6 +480,10 @@ static int parse_vlan(struct sk_buff *skb, struct
sw_flow_key *key)
};
struct qtag_prefix *qp;
+ if (unlikely(skb->len < sizeof(struct qtag_prefix) +
+ sizeof(__be16)))
+ return 0;
+
if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
sizeof(__be16))))
return -ENOMEM;
@@ -949,9 +953,13 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int
*key_lenp,
/* Only standard 0x8100 VLANs currently supported. */
if (q_key->q_tpid != htons(ETH_P_8021Q))
goto invalid;
- if (q_key->q_tci & htons(VLAN_TAG_PRESENT))
- goto invalid;
- swkey->eth.tci = q_key->q_tci | htons(VLAN_TAG_PRESENT);
+ if (q_key->q_tci & htons(VLAN_TAG_PRESENT)) {
+ swkey->eth.tci = q_key->q_tci;
+ } else {
+ if (q_key->q_tci)
+ goto invalid;
+ swkey->eth.type = q_key->q_tpid;
+ }
break;
case TRANSITION(OVS_KEY_ATTR_8021Q, OVS_KEY_ATTR_ETHERTYPE):
@@ -959,6 +967,9 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int
*key_lenp,
swkey->eth.type = nla_get_be16(nla);
if (ntohs(swkey->eth.type) < 1536)
goto invalid;
+ if (swkey->eth.type == htons(ETH_P_8021Q) &&
+ !(swkey->eth.tci & htons(VLAN_TAG_PRESENT)))
+ goto invalid;
break;
case TRANSITION(OVS_KEY_ATTR_ETHERTYPE, OVS_KEY_ATTR_IPV4):
@@ -1232,12 +1243,16 @@ int flow_to_nlattrs(const struct sw_flow_key *swkey,
struct sk_buff *skb)
memcpy(eth_key->eth_src, swkey->eth.src, ETH_ALEN);
memcpy(eth_key->eth_dst, swkey->eth.dst, ETH_ALEN);
- if (swkey->eth.tci != htons(0)) {
+ if (swkey->eth.tci & htons(VLAN_TAG_PRESENT) ||
+ swkey->eth.type == htons(ETH_P_8021Q)) {
struct ovs_key_8021q q_key;
q_key.q_tpid = htons(ETH_P_8021Q);
- q_key.q_tci = swkey->eth.tci & ~htons(VLAN_TAG_PRESENT);
+ q_key.q_tci = swkey->eth.tci;
NLA_PUT(skb, OVS_KEY_ATTR_8021Q, sizeof(q_key), &q_key);
+
+ if (!(swkey->eth.tci & htons(VLAN_TAG_PRESENT)))
+ return 0;
}
if (swkey->eth.type == htons(ETH_P_802_2))
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b4e788f..101d129 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1305,7 +1305,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
nested = nl_attr_get(a);
assert(nl_attr_type(nested) == OVS_KEY_ATTR_8021Q);
q_key = nl_attr_get_unspec(nested, sizeof(*q_key));
- eth_push_vlan(packet, q_key->q_tci);
+ eth_push_vlan(packet, q_key->q_tci & ~htons(VLAN_CFI));
break;
case OVS_ACTION_ATTR_POP:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 1e9289a..5a4eda4 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -585,7 +585,8 @@ parse_odp_key_attr(const char *s, struct ofpbuf *key)
q_key.q_tpid = htons(tpid);
q_key.q_tci = htons((vid << VLAN_VID_SHIFT) |
- (pcp << VLAN_PCP_SHIFT));
+ (pcp << VLAN_PCP_SHIFT) |
+ VLAN_CFI);
nl_msg_put_unspec(key, OVS_KEY_ATTR_8021Q, &q_key, sizeof q_key);
return n;
}
@@ -850,13 +851,18 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct
flow *flow)
memcpy(eth_key->eth_src, flow->dl_src, ETH_ADDR_LEN);
memcpy(eth_key->eth_dst, flow->dl_dst, ETH_ADDR_LEN);
- if (flow->vlan_tci != htons(0)) {
+ if (flow->vlan_tci & htons(VLAN_CFI) ||
+ flow->dl_type == htons(ETH_TYPE_VLAN)) {
struct ovs_key_8021q *q_key;
q_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_8021Q,
sizeof *q_key);
q_key->q_tpid = htons(ETH_TYPE_VLAN);
- q_key->q_tci = flow->vlan_tci & ~htons(VLAN_CFI);
+ q_key->q_tci = flow->vlan_tci;
+
+ if (!(flow->vlan_tci & htons(VLAN_CFI))) {
+ return;
+ }
}
if (ntohs(flow->dl_type) < ETH_TYPE_MIN) {
@@ -1037,9 +1043,13 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t
key_len,
return EINVAL;
}
if (q_key->q_tci & htons(VLAN_CFI)) {
- return EINVAL;
+ flow->vlan_tci = q_key->q_tci;
+ } else {
+ if (q_key->q_tci) {
+ return EINVAL;
+ }
+ flow->dl_type = q_key->q_tpid;
}
- flow->vlan_tci = q_key->q_tci | htons(VLAN_CFI);
break;
case TRANSITION(OVS_KEY_ATTR_8021Q, OVS_KEY_ATTR_ETHERTYPE):
@@ -1048,6 +1058,10 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t
key_len,
if (ntohs(flow->dl_type) < 1536) {
return EINVAL;
}
+ if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
+ !(flow->vlan_tci & htons(VLAN_CFI))) {
+ return EINVAL;
+ }
break;
case TRANSITION(OVS_KEY_ATTR_ETHERTYPE, OVS_KEY_ATTR_IPV4):
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6c303bb..29f0ade 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3648,7 +3648,7 @@ commit_vlan_action(struct action_xlate_ctx *ctx, ovs_be16
new_tci)
struct ovs_key_8021q q_key;
q_key.q_tpid = htons(ETH_TYPE_VLAN);
- q_key.q_tci = new_tci & ~htons(VLAN_CFI);
+ q_key.q_tci = new_tci;
commit_action__(ctx->odp_actions, OVS_ACTION_ATTR_PUSH,
OVS_KEY_ATTR_8021Q, &q_key, sizeof(q_key));
@@ -4731,6 +4731,14 @@ flow_get_vlan(struct ofproto_dpif *ofproto, const struct
flow *flow,
return -1;
}
} else {
+ if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
+ !(flow->vlan_tci & htons(VLAN_CFI))) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_WARN_RL(&rl, "bridge %s: dropping packet with partial "
+ "VLAN tag received on port %s",
+ ofproto->up.name, in_bundle->name);
+ return -1;
+ }
if (in_bundle->vlan_mode != PORT_VLAN_TRUNK) {
return in_bundle->vlan;
} else {
--
1.7.5.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
