From patchwork Tue Dec 13 00:12:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TWljaGHFgiBNaXJvc8WCYXc=?= X-Patchwork-Id: 705254 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3td0Y15LXbz9t0J for ; Tue, 13 Dec 2016 11:13:25 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 73428C0A; Tue, 13 Dec 2016 00:12:47 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 9643EBE4 for ; Tue, 13 Dec 2016 00:12:46 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from rere.qmqm.pl (rere.qmqm.pl [84.10.57.10]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 692D4204 for ; Tue, 13 Dec 2016 00:12:45 +0000 (UTC) Received: by rere.qmqm.pl (Postfix, from userid 1000) id 6695F6119; Tue, 13 Dec 2016 01:12:38 +0100 (CET) Message-Id: In-Reply-To: References: From: =?UTF-8?q?Micha=C5=82=20Miros=C5=82aw?= MIME-Version: 1.0 To: netdev@vger.kernel.org Date: Tue, 13 Dec 2016 01:12:38 +0100 (CET) X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: "open list:OPENVSWITCH" Subject: [ovs-dev] [PATCH net-next 17/27] OVS: remove assumptions about VLAN_TAG_PRESENT bit X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This leaves CFI bit toggled in API, because userspace might depend this is set for normal ethernet traffic with tag present. Signed-off-by: Michał Mirosław --- Documentation/networking/openvswitch.txt | 14 -------- net/openvswitch/actions.c | 13 +++---- net/openvswitch/flow.c | 4 +-- net/openvswitch/flow.h | 4 +-- net/openvswitch/flow_netlink.c | 61 ++++++++++---------------------- 5 files changed, 30 insertions(+), 66 deletions(-) diff --git a/Documentation/networking/openvswitch.txt b/Documentation/networking/openvswitch.txt index b3b9ac6..e7ca27d 100644 --- a/Documentation/networking/openvswitch.txt +++ b/Documentation/networking/openvswitch.txt @@ -219,20 +219,6 @@ 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/net/openvswitch/actions.c b/net/openvswitch/actions.c index 514f7bc..6015bc9 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -277,8 +277,7 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key, key->eth.vlan.tci = vlan->vlan_tci; key->eth.vlan.tpid = vlan->vlan_tpid; } - return skb_vlan_push(skb, vlan->vlan_tpid, - ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT); + return skb_vlan_push(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci ^ VLAN_CFI_MASK)); } /* 'src' is already properly masked. */ @@ -704,8 +703,10 @@ static int ovs_vport_output(struct net *net, struct sock *sk, struct sk_buff *sk __skb_dst_copy(skb, data->dst); *OVS_CB(skb) = data->cb; skb->inner_protocol = data->inner_protocol; - skb->vlan_tci = data->vlan_tci; - skb->vlan_proto = data->vlan_proto; + if (data->vlan_proto) + __vlan_hwaccel_put_tag(skb, data->vlan_proto, data->vlan_tci ^ VLAN_CFI_MASK); + else + __vlan_hwaccel_clear_tag(skb); /* Reconstruct the MAC header. */ skb_push(skb, data->l2_len); @@ -749,8 +750,8 @@ static void prepare_frag(struct vport *vport, struct sk_buff *skb, data->cb = *OVS_CB(skb); data->inner_protocol = skb->inner_protocol; data->network_offset = orig_network_offset; - data->vlan_tci = skb->vlan_tci; - data->vlan_proto = skb->vlan_proto; + data->vlan_tci = skb_vlan_tag_present(skb) ? skb->vlan_tci ^ VLAN_CFI_MASK : 0; + data->vlan_proto = skb_vlan_tag_present(skb) ? skb->vlan_proto : 0; data->mac_proto = mac_proto; data->l2_len = hlen; memcpy(&data->l2_data, skb->data, hlen); diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 08aa926..df58cfd 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -327,7 +327,7 @@ static int parse_vlan_tag(struct sk_buff *skb, struct vlan_head *key_vh) return -ENOMEM; vh = (struct vlan_head *)skb->data; - key_vh->tci = vh->tci | htons(VLAN_TAG_PRESENT); + key_vh->tci = vh->tci ^ htons(VLAN_CFI_MASK); key_vh->tpid = vh->tpid; __skb_pull(skb, sizeof(struct vlan_head)); @@ -347,7 +347,7 @@ static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key) int res; if (skb_vlan_tag_present(skb)) { - key->eth.vlan.tci = htons(skb->vlan_tci); + key->eth.vlan.tci = htons(skb->vlan_tci) ^ htons(VLAN_CFI_MASK); key->eth.vlan.tpid = skb->vlan_proto; } else { /* Parse outer vlan tag in the non-accelerated case. */ diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h index f61cae7..f5115ed 100644 --- a/net/openvswitch/flow.h +++ b/net/openvswitch/flow.h @@ -57,8 +57,8 @@ struct ovs_tunnel_info { }; struct vlan_head { - __be16 tpid; /* Vlan type. Generally 802.1q or 802.1ad.*/ - __be16 tci; /* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */ + __be16 tpid; /* Vlan type. Generally 802.1q or 802.1ad. 0 if no VLAN*/ + __be16 tci; }; #define OVS_SW_FLOW_KEY_METADATA_SIZE \ diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c index d19044f..6ae5218 100644 --- a/net/openvswitch/flow_netlink.c +++ b/net/openvswitch/flow_netlink.c @@ -835,8 +835,6 @@ static int validate_vlan_from_nlattrs(const struct sw_flow_match *match, u64 key_attrs, bool inner, const struct nlattr **a, bool log) { - __be16 tci = 0; - if (!((key_attrs & (1 << OVS_KEY_ATTR_ETHERNET)) && (key_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) && eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE])))) { @@ -850,20 +848,11 @@ static int validate_vlan_from_nlattrs(const struct sw_flow_match *match, return -EINVAL; } - if (a[OVS_KEY_ATTR_VLAN]) - tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]); - - if (!(tci & htons(VLAN_TAG_PRESENT))) { - if (tci) { - OVS_NLERR(log, "%s TCI does not have VLAN_TAG_PRESENT bit set.", - (inner) ? "C-VLAN" : "VLAN"); - return -EINVAL; - } else if (nla_len(a[OVS_KEY_ATTR_ENCAP])) { - /* Corner case for truncated VLAN header. */ - OVS_NLERR(log, "Truncated %s header has non-zero encap attribute.", - (inner) ? "C-VLAN" : "VLAN"); - return -EINVAL; - } + if (!a[OVS_KEY_ATTR_VLAN] && nla_len(a[OVS_KEY_ATTR_ENCAP])) { + /* Corner case for truncated VLAN header. */ + OVS_NLERR(log, "Truncated %s header has non-zero encap attribute.", + (inner) ? "C-VLAN" : "VLAN"); + return -EINVAL; } return 1; @@ -873,12 +862,9 @@ static int validate_vlan_mask_from_nlattrs(const struct sw_flow_match *match, u64 key_attrs, bool inner, const struct nlattr **a, bool log) { - __be16 tci = 0; __be16 tpid = 0; - bool encap_valid = !!(match->key->eth.vlan.tci & - htons(VLAN_TAG_PRESENT)); - bool i_encap_valid = !!(match->key->eth.cvlan.tci & - htons(VLAN_TAG_PRESENT)); + bool encap_valid = !!match->key->eth.vlan.tpid; + bool i_encap_valid = !!match->key->eth.cvlan.tpid; if (!(key_attrs & (1 << OVS_KEY_ATTR_ENCAP))) { /* Not a VLAN. */ @@ -891,9 +877,6 @@ static int validate_vlan_mask_from_nlattrs(const struct sw_flow_match *match, return -EINVAL; } - if (a[OVS_KEY_ATTR_VLAN]) - tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]); - if (a[OVS_KEY_ATTR_ETHERTYPE]) tpid = nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]); @@ -902,11 +885,6 @@ static int validate_vlan_mask_from_nlattrs(const struct sw_flow_match *match, (inner) ? "C-VLAN" : "VLAN", ntohs(tpid)); return -EINVAL; } - if (!(tci & htons(VLAN_TAG_PRESENT))) { - OVS_NLERR(log, "%s TCI mask does not have exact match for VLAN_TAG_PRESENT bit.", - (inner) ? "C-VLAN" : "VLAN"); - return -EINVAL; - } return 1; } @@ -958,7 +936,7 @@ static int parse_vlan_from_nlattrs(struct sw_flow_match *match, if (err) return err; - encap_valid = !!(match->key->eth.vlan.tci & htons(VLAN_TAG_PRESENT)); + encap_valid = !!match->key->eth.vlan.tpid; if (encap_valid) { err = __parse_vlan_from_nlattrs(match, key_attrs, true, a, is_mask, log); @@ -1974,12 +1952,12 @@ static inline void add_nested_action_end(struct sw_flow_actions *sfa, static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, const struct sw_flow_key *key, int depth, struct sw_flow_actions **sfa, - __be16 eth_type, __be16 vlan_tci, bool log); + __be16 eth_type, __be16 vlan_tci, bool has_vlan, bool log); static int validate_and_copy_sample(struct net *net, const struct nlattr *attr, const struct sw_flow_key *key, int depth, struct sw_flow_actions **sfa, - __be16 eth_type, __be16 vlan_tci, bool log) + __be16 eth_type, __be16 vlan_tci, bool has_vlan, bool log) { const struct nlattr *attrs[OVS_SAMPLE_ATTR_MAX + 1]; const struct nlattr *probability, *actions; @@ -2017,7 +1995,7 @@ static int validate_and_copy_sample(struct net *net, const struct nlattr *attr, return st_acts; err = __ovs_nla_copy_actions(net, actions, key, depth + 1, sfa, - eth_type, vlan_tci, log); + eth_type, vlan_tci, has_vlan, log); if (err) return err; @@ -2358,7 +2336,7 @@ static int copy_action(const struct nlattr *from, static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, const struct sw_flow_key *key, int depth, struct sw_flow_actions **sfa, - __be16 eth_type, __be16 vlan_tci, bool log) + __be16 eth_type, __be16 vlan_tci, bool has_vlan, bool log) { u8 mac_proto = ovs_key_mac_proto(key); const struct nlattr *a; @@ -2436,6 +2414,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, if (mac_proto != MAC_PROTO_ETHERNET) return -EINVAL; vlan_tci = htons(0); + has_vlan = 0; break; case OVS_ACTION_ATTR_PUSH_VLAN: @@ -2444,9 +2423,8 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, vlan = nla_data(a); if (!eth_type_vlan(vlan->vlan_tpid)) return -EINVAL; - if (!(vlan->vlan_tci & htons(VLAN_TAG_PRESENT))) - return -EINVAL; vlan_tci = vlan->vlan_tci; + has_vlan = 1; break; case OVS_ACTION_ATTR_RECIRC: @@ -2460,7 +2438,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, /* Prohibit push MPLS other than to a white list * for packets that have a known tag order. */ - if (vlan_tci & htons(VLAN_TAG_PRESENT) || + if (has_vlan || (eth_type != htons(ETH_P_IP) && eth_type != htons(ETH_P_IPV6) && eth_type != htons(ETH_P_ARP) && @@ -2472,8 +2450,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, } case OVS_ACTION_ATTR_POP_MPLS: - if (vlan_tci & htons(VLAN_TAG_PRESENT) || - !eth_p_mpls(eth_type)) + if (has_vlan || !eth_p_mpls(eth_type)) return -EINVAL; /* Disallow subsequent L2.5+ set and mpls_pop actions @@ -2506,7 +2483,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, case OVS_ACTION_ATTR_SAMPLE: err = validate_and_copy_sample(net, a, key, depth, sfa, - eth_type, vlan_tci, log); + eth_type, vlan_tci, has_vlan, log); if (err) return err; skip_copy = true; @@ -2530,7 +2507,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, case OVS_ACTION_ATTR_POP_ETH: if (mac_proto != MAC_PROTO_ETHERNET) return -EINVAL; - if (vlan_tci & htons(VLAN_TAG_PRESENT)) + if (has_vlan) return -EINVAL; mac_proto = MAC_PROTO_ETHERNET; break; @@ -2565,7 +2542,7 @@ int ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, (*sfa)->orig_len = nla_len(attr); err = __ovs_nla_copy_actions(net, attr, key, 0, sfa, key->eth.type, - key->eth.vlan.tci, log); + key->eth.vlan.tci, !!key->eth.vlan.tpid, log); if (err) ovs_nla_free_flow_actions(*sfa);