[ovs-dev] [PATCH v2 14/17] datapath: Fix skb->protocol for vlan frames
Yang, Yi
yi.y.yang at intel.com
Thu Jan 19 00:53:05 UTC 2017
On Wed, Jan 18, 2017 at 01:29:14PM -0800, Joe Stringer wrote:
> On 18 January 2017 at 11:54, Eric Garver <e at erig.me> wrote:
> > On Tue, Jan 17, 2017 at 12:37:19AM +0000, Yang, Yi Y wrote:
> >> What userspace do "802.1ad patches" depend on? Per Pravin's statement, we just backport 802.1ad patches to ovs, then the below patch can be applied to ovs.
> >
> > Userspace does not yet have support for 802.1ad. I'm still working on
> > it. You can check the list archives for a recent RFC version.
> >
> > I don't know if it's acceptable to backport the datapath (kernel module)
> > support before the userspace support is accepted. If not, you'll have to
> > wait on the userspace.
> > Perhaps Pravin can answer.
>
> IMO the general method of:
> 1) Add support upstream
> 2) Add userspace support
> 3) Add backport
>
> ...works nicely because we get feedback for all interested parties for
> the APIs in (1), (2) can add tests and be easily tested against a
> version that works (upstream kernel) and a version that doesn't
> (version in tree) to ensure both cases are handled in a reasonable
> way, then (3) allows people on older kernels to gain access to the
> newer features.
>
> That said, if other people are blocking on (3) then I think that piece
> should be expedited. Let's say (2) and (3) were swapped, it just means
> we need to be a bit more careful when reviewing/testing to check that
> the newer userspace still handles old kernels (that lack support)
> fine.
>
> The nice thing about getting the backport earlier is, the closer to
> upstream we are, the sooner we may find issues that affect the latest
> code.
If so, I think the below patch is the best solution to current
situation, it will be automatically overwritten once userspace part and
802.1ad backport are ready to merge.
So I propose we can merge this patch serial first, then merge Eric
Garver's rtnetlink patch serial. By then, ovs can completely support L3
tunnel ports in both compat mode and kernel datapath mode, it will also
work well on Linux 4.7 and later versions.
>From 127b1c7af10220dacc5b0c6a7c8b4e19d9d4ecac Mon Sep 17 00:00:00 2001
From: Yi Yang <yi.y.yang at intel.com>
Date: Wed, 28 Dec 2016 10:27:11 +0800
Subject: [PATCH v2 14/17] datapath: Fix skb->protocol for vlan frames
Do not always set skb->protocol to be the ethertype of the L3 header.
For a packet with non-accelerated VLAN tags skb->protocol needs to be the ethertype of the outermost non-accelerated VLAN ethertype.
Any VLAN offloading is undone on the OVS netlink interface, and any VLAN tags added by userspace are non-accelerated, as are double tagged VLAN packets.
Fixes: 018c1dda5f ("openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink attributes")
Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Yi Yang <yi.y.yang at intel.com>
---
datapath/datapath.c | 1 -
datapath/flow.c | 42 +++++++++++++++++-------------------------
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 1deb62d..dd19a0e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -617,7 +617,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
packet->priority = flow->key.phy.priority;
packet->mark = flow->key.phy.skb_mark;
- packet->protocol = flow->key.eth.type;
rcu_read_lock();
dp = get_dp_rcu(net, ovs_header->dp_ifindex);
diff --git a/datapath/flow.c b/datapath/flow.c
index 240bd00..cf35272 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -334,6 +334,7 @@ static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
qp = (struct qtag_prefix *) skb->data;
key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
+ key->eth.type = qp->eth_type;
__skb_pull(skb, sizeof(struct qtag_prefix));
return 0;
@@ -493,6 +494,7 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return -EINVAL;
skb_reset_network_header(skb);
+ key->eth.type = skb->protocol;
} else {
eth = eth_hdr(skb);
ether_addr_copy(key->eth.src, eth->h_source);
@@ -504,11 +506,14 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
*/
key->eth.tci = 0;
- if (skb_vlan_tag_present(skb))
+ if (skb_vlan_tag_present(skb)) {
key->eth.tci = htons(skb->vlan_tci);
+ key->eth.type = skb->vlan_proto;
+ }
else if (eth->h_proto == htons(ETH_P_8021Q))
if (unlikely(parse_vlan(skb, key)))
return -ENOMEM;
+ skb->protocol = key->eth.type;
key->eth.type = parse_ethertype(skb);
if (unlikely(key->eth.type == htons(0)))
@@ -519,7 +524,8 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
}
skb_reset_mac_len(skb);
- key->eth.type = skb->protocol;
+ if (!eth_type_is_vlan(skb->protocol))
+ skb->protocol = key->eth.type;
/* Network layer. */
if (key->eth.type == htons(ETH_P_IP)) {
@@ -786,29 +792,15 @@ int ovs_flow_key_extract_userspace(struct net *net, const struct nlattr *attr,
if (err)
return err;
- if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
- /* key_extract assumes that skb->protocol is set-up for
- * layer 3 packets which is the case for other callers,
- * in particular packets recieved from the network stack.
- * Here the correct value can be set from the metadata
- * extracted above.
- */
- skb->protocol = key->eth.type;
- } else {
- struct ethhdr *eth;
-
- skb_reset_mac_header(skb);
- eth = eth_hdr(skb);
-
- /* Normally, setting the skb 'protocol' field would be
- * handled by a call to eth_type_trans(), but it assumes
- * there's a sending device, which we may not have.
- */
- if (eth_proto_is_802_3(eth->h_proto))
- skb->protocol = eth->h_proto;
- else
- skb->protocol = htons(ETH_P_802_2);
- }
+ /* key_extract assumes that skb->protocol is set-up for
+ * layer 3 packets which is the case for other callers,
+ * in particular packets recieved from the network stack.
+ * Here the correct value can be set from the metadata
+ * extracted above. For layer 2 packets we initialize
+ * skb->protocol to zero and set it in key_extract() while
+ * parsing the L2 headers.
+ */
+ skb->protocol = key->eth.type;
return key_extract(skb, key);
}
--
2.1.0
More information about the dev
mailing list