aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--datapath/README35
-rw-r--r--datapath/actions.c2
-rw-r--r--datapath/datapath.c2
-rw-r--r--datapath/flow.c45
-rw-r--r--include/linux/openvswitch.h4
-rw-r--r--lib/dpif-netdev.c2
-rw-r--r--lib/odp-util.c94
-rw-r--r--ofproto/ofproto-dpif.c10
8 files changed, 150 insertions, 44 deletions
diff --git a/datapath/README b/datapath/README
index 2d8a141b..b8a048b8 100644
--- a/datapath/README
+++ b/datapath/README
@@ -145,6 +145,41 @@ not understand the "vlan" key will not see either of those attributes
and therefore will not misinterpret them. (Also, the outer eth_type
is still 0x8100, not changed to 0x0800.)
+Handling malformed packets
+--------------------------
+
+Don't drop packets in the kernel for malformed protocol headers, bad
+checksums, etc. This would prevent userspace from implementing a
+simple Ethernet switch that forwards every packet.
+
+Instead, in such a case, include an attribute with "empty" content.
+It doesn't matter if the empty content could be valid protocol values,
+as long as those values are rarely seen in practice, because userspace
+can always forward all packets with those values to userspace and
+handle them individually.
+
+For example, consider a packet that contains an IP header that
+indicates protocol 6 for TCP, but which is truncated just after the IP
+header, so that the TCP header is missing. The flow key for this
+packet would include a tcp attribute with all-zero src and dst, like
+this:
+
+ eth(...), eth_type(0x0800), ip(proto=6, ...), tcp(src=0, dst=0)
+
+As another example, consider a packet with an Ethernet type of 0x8100,
+indicating that a VLAN TCI should follow, but which is truncated just
+after the Ethernet type. The flow key for this packet would include
+an all-zero-bits vlan and an empty encap attribute, like this:
+
+ eth(...), eth_type(0x8100), vlan(0), encap()
+
+Unlike a TCP packet with source and destination ports 0, an
+all-zero-bits VLAN TCI is not that rare, so the CFI bit (aka
+VLAN_TAG_PRESENT inside the kernel) is ordinarily set in a vlan
+attribute expressly to allow this situation to be distinguished.
+Thus, the flow key in this second example unambiguously indicates a
+missing or malformed VLAN TCI.
+
Other rules
-----------
diff --git a/datapath/actions.c b/datapath/actions.c
index 88eca6cd..03fef922 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -112,7 +112,7 @@ static int push_vlan(struct sk_buff *skb, const struct ovs_action_push_vlan *vla
+ ETH_HLEN, VLAN_HLEN, 0));
}
- __vlan_hwaccel_put_tag(skb, ntohs(vlan->vlan_tci));
+ __vlan_hwaccel_put_tag(skb, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
return 0;
}
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 2d67657d..c7d24129 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -657,7 +657,7 @@ static int validate_actions(const struct nlattr *attr,
vlan = nla_data(a);
if (vlan->vlan_tpid != htons(ETH_P_8021Q))
return -EINVAL;
- if (vlan->vlan_tci & htons(VLAN_TAG_PRESENT))
+ if (!(vlan->vlan_tci & htons(VLAN_TAG_PRESENT)))
return -EINVAL;
break;
diff --git a/datapath/flow.c b/datapath/flow.c
index fded9375..7a136248 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -480,6 +480,9 @@ 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;
@@ -1045,18 +1048,35 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
memcpy(swkey->eth.src, eth_key->eth_src, ETH_ALEN);
memcpy(swkey->eth.dst, eth_key->eth_dst, ETH_ALEN);
- if (attrs == ((1 << OVS_KEY_ATTR_VLAN) |
- (1 << OVS_KEY_ATTR_ETHERTYPE) |
- (1 << OVS_KEY_ATTR_ENCAP)) &&
+ if (attrs & (1u << OVS_KEY_ATTR_ETHERTYPE) &&
nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) == htons(ETH_P_8021Q)) {
- swkey->eth.tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
- if (swkey->eth.tci & htons(VLAN_TAG_PRESENT))
+ const struct nlattr *encap;
+ __be16 tci;
+
+ if (attrs != ((1 << OVS_KEY_ATTR_VLAN) |
+ (1 << OVS_KEY_ATTR_ETHERTYPE) |
+ (1 << OVS_KEY_ATTR_ENCAP)))
return -EINVAL;
- swkey->eth.tci |= htons(VLAN_TAG_PRESENT);
- err = parse_flow_nlattrs(a[OVS_KEY_ATTR_ENCAP], a, &attrs);
- if (err)
- return err;
+ encap = a[OVS_KEY_ATTR_ENCAP];
+ tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
+ if (tci & htons(VLAN_TAG_PRESENT)) {
+ swkey->eth.tci = tci;
+
+ err = parse_flow_nlattrs(encap, a, &attrs);
+ if (err)
+ return err;
+ } else if (!tci) {
+ /* Corner case for truncated 802.1Q header. */
+ if (nla_len(encap))
+ return -EINVAL;
+
+ swkey->eth.type = htons(ETH_P_8021Q);
+ *key_lenp = key_len;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}
if (attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) {
@@ -1214,11 +1234,12 @@ 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 || swkey->eth.type == htons(ETH_P_8021Q)) {
NLA_PUT_BE16(skb, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_P_8021Q));
- NLA_PUT_BE16(skb, OVS_KEY_ATTR_VLAN,
- swkey->eth.tci & ~htons(VLAN_TAG_PRESENT));
+ NLA_PUT_BE16(skb, OVS_KEY_ATTR_VLAN, swkey->eth.tci);
encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP);
+ if (!swkey->eth.tci)
+ goto unencap;
} else {
encap = NULL;
}
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index 2d141e0d..4f081369 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -439,8 +439,8 @@ enum ovs_userspace_attr {
/**
* struct ovs_action_push_vlan - %OVS_ACTION_ATTR_PUSH_VLAN action argument.
* @vlan_tpid: Tag protocol identifier (TPID) to push.
- * @vlan_tci: Tag control identifier (TCI) to push. The CFI bit must not be
- * set.
+ * @vlan_tci: Tag control identifier (TCI) to push. The CFI bit must be set
+ * (but it will not be set in the 802.1Q header that is pushed).
*
* The @vlan_tpid value is typically %ETH_P_8021Q. The only acceptable TPID
* values are those that the kernel module also parses as 802.1Q headers, to
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index eb8d66d9..bc93a102 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1313,7 +1313,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
case OVS_ACTION_ATTR_PUSH_VLAN:
vlan = nl_attr_get(a);
- eth_push_vlan(packet, vlan->vlan_tci);
+ eth_push_vlan(packet, vlan->vlan_tci & ~htons(VLAN_CFI));
break;
case OVS_ACTION_ATTR_POP_VLAN:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index b974dcc4..38215538 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -198,6 +198,17 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
}
static void
+format_vlan_tci(struct ds *ds, ovs_be16 vlan_tci)
+{
+ ds_put_format(ds, "vid=%"PRIu16",pcp=%d",
+ vlan_tci_to_vid(vlan_tci),
+ vlan_tci_to_pcp(vlan_tci));
+ if (!(vlan_tci & htons(VLAN_CFI))) {
+ ds_put_cstr(ds, ",cfi=0");
+ }
+}
+
+static void
format_odp_action(struct ds *ds, const struct nlattr *a)
{
int expected_len;
@@ -230,9 +241,8 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
if (vlan->vlan_tpid != htons(ETH_TYPE_VLAN)) {
ds_put_format(ds, "tpid=0x%04"PRIx16",", ntohs(vlan->vlan_tpid));
}
- ds_put_format(ds, "vid=%"PRIu16",pcp=%d)",
- vlan_tci_to_vid(vlan->vlan_tci),
- vlan_tci_to_pcp(vlan->vlan_tci));
+ format_vlan_tci(ds, vlan->vlan_tci);
+ ds_put_char(ds, ')');
break;
case OVS_ACTION_ATTR_POP_VLAN:
ds_put_cstr(ds, "pop_vlan");
@@ -395,9 +405,9 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
break;
case OVS_KEY_ATTR_VLAN:
- ds_put_format(ds, "(vid=%"PRIu16",pcp=%d)",
- vlan_tci_to_vid(nl_attr_get_be16(a)),
- vlan_tci_to_pcp(nl_attr_get_be16(a)));
+ ds_put_char(ds, '(');
+ format_vlan_tci(ds, nl_attr_get_be16(a));
+ ds_put_char(ds, ')');
break;
case OVS_KEY_ATTR_ETHERTYPE:
@@ -616,13 +626,23 @@ parse_odp_key_attr(const char *s, struct ofpbuf *key)
{
uint16_t vid;
int pcp;
+ int cfi;
int n = -1;
if ((sscanf(s, "vlan(vid=%"SCNi16",pcp=%i)%n", &vid, &pcp, &n) > 0
&& n > 0)) {
nl_msg_put_be16(key, OVS_KEY_ATTR_VLAN,
htons((vid << VLAN_VID_SHIFT) |
- (pcp << VLAN_PCP_SHIFT)));
+ (pcp << VLAN_PCP_SHIFT) |
+ VLAN_CFI));
+ return n;
+ } else if ((sscanf(s, "vlan(vid=%"SCNi16",pcp=%i,cfi=%i)%n",
+ &vid, &pcp, &cfi, &n) > 0
+ && n > 0)) {
+ nl_msg_put_be16(key, OVS_KEY_ATTR_VLAN,
+ htons((vid << VLAN_VID_SHIFT) |
+ (pcp << VLAN_PCP_SHIFT) |
+ (cfi ? VLAN_CFI : 0)));
return n;
}
}
@@ -920,11 +940,13 @@ 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(0) || flow->dl_type == htons(ETH_TYPE_VLAN)) {
nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_TYPE_VLAN));
- nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN,
- flow->vlan_tci & ~htons(VLAN_CFI));
+ nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, flow->vlan_tci);
encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
+ if (flow->vlan_tci == htons(0)) {
+ goto unencap;
+ }
} else {
encap = 0;
}
@@ -1198,27 +1220,47 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
}
expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ETHERNET;
- if ((present_attrs & ~expected_attrs)
- == ((UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE) |
- (UINT64_C(1) << OVS_KEY_ATTR_VLAN) |
- (UINT64_C(1) << OVS_KEY_ATTR_ENCAP))
+ if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE)
&& (nl_attr_get_be16(attrs[OVS_KEY_ATTR_ETHERTYPE])
== htons(ETH_TYPE_VLAN))) {
- const struct nlattr *encap = attrs[OVS_KEY_ATTR_ENCAP];
- const struct nlattr *vlan = attrs[OVS_KEY_ATTR_VLAN];
-
- flow->vlan_tci = nl_attr_get_be16(vlan);
- if (flow->vlan_tci & htons(VLAN_CFI)) {
- return EINVAL;
- }
- flow->vlan_tci |= htons(VLAN_CFI);
-
- error = parse_flow_nlattrs(nl_attr_get(encap), nl_attr_get_size(encap),
- attrs, &present_attrs);
+ /* The Ethernet type is 0x8100 so there must be a VLAN tag
+ * and encapsulated protocol information. */
+ const struct nlattr *encap;
+ __be16 tci;
+ int error;
+
+ expected_attrs |= ((UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE) |
+ (UINT64_C(1) << OVS_KEY_ATTR_VLAN) |
+ (UINT64_C(1) << OVS_KEY_ATTR_ENCAP));
+ error = check_expectations(present_attrs, expected_attrs,
+ key, key_len);
if (error) {
return error;
}
- expected_attrs = 0;
+
+ encap = attrs[OVS_KEY_ATTR_ENCAP];
+ tci = nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]);
+ if (tci & htons(VLAN_CFI)) {
+ flow->vlan_tci = tci;
+
+ error = parse_flow_nlattrs(nl_attr_get(encap),
+ nl_attr_get_size(encap),
+ attrs, &present_attrs);
+ if (error) {
+ return error;
+ }
+ expected_attrs = 0;
+ } else if (tci == htons(0)) {
+ /* Corner case for a truncated 802.1Q header. */
+ if (nl_attr_get_size(encap)) {
+ return EINVAL;
+ }
+
+ flow->dl_type = htons(ETH_TYPE_VLAN);
+ return 0;
+ } else {
+ return EINVAL;
+ }
}
if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE)) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 9356800c..d525d4e7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3638,7 +3638,7 @@ commit_vlan_action(struct action_xlate_ctx *ctx, ovs_be16 new_tci)
struct ovs_action_push_vlan vlan;
vlan.vlan_tpid = htons(ETH_TYPE_VLAN);
- vlan.vlan_tci = new_tci & ~htons(VLAN_CFI);
+ vlan.vlan_tci = new_tci;
nl_msg_put_unspec(ctx->odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
&vlan, sizeof vlan);
}
@@ -4719,6 +4719,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 {
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-15 16:20:30 +0000