On Tue, Nov 29, 2016 at 3:30 PM, Jarno Rajahalme <ja...@ovn.org> wrote: > 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 <ja...@ovn.org>
Looks much better now. Thanks for fixing it. I have one minor comment. Acked-by: Pravin B Shelar <pshe...@ovn.org> > --- > v3: Set skb->protocol properly also for double tagged packets, as suggested > by Pravin. This patch is no longer needed for the MTU test failure, as > the new patch 2/3 addresses that. > > net/openvswitch/datapath.c | 1 - > net/openvswitch/flow.c | 62 > +++++++++++++++++++++++----------------------- > 2 files changed, 31 insertions(+), 32 deletions(-) > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 08aa926..e2fe2e5 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -354,6 +354,7 @@ static int parse_vlan(struct sk_buff *skb, struct > sw_flow_key *key) > res = parse_vlan_tag(skb, &key->eth.vlan); > if (res <= 0) > return res; > + skb->protocol = key->eth.vlan.tpid; > } > > /* Parse inner vlan tag. */ > @@ -361,6 +362,11 @@ static int parse_vlan(struct sk_buff *skb, struct > sw_flow_key *key) > if (res <= 0) > return res; > > + /* If the outer vlan tag was accelerated, skb->protocol should > + * refelect the inner vlan type. */ > + if (!eth_type_vlan(skb->protocol)) Since you would be spinning another version, can you change this condition to directly check for skb-vlan-tag rather than indirectly checking for the vlan accelerated case?