Message ID | 76814927-D373-4C3A-BC85-5771304235A7@ovn.org |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, 28 Nov 2016 14:29:39 -0800, Jarno Rajahalme wrote: > I’m not sure what you suggest here. Obviously the kernel ABI can not > be changed as existing userspace code expects upcalled packets to be > non-accelerated. Also, if userspace pushes vlan headers, the packet > will actually have them. The user space API needs to be preserved, of course. I'm talking about what happens internally in the kernel. See this patchset: https://www.spinics.net/lists/netdev/msg398827.html > Would this incremental fix this: > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 9be9fda..37f1bb9 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -354,6 +354,8 @@ 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; > + if (skb->protocol == htons(ETH_P_TEB)) > + skb->protocol = key->eth.vlan.tpid; > } > > /* Parse inner vlan tag. */ I'll look at this tomorrow. But it seems we're adding more and more hacks instead of cleaning up the vlan handling. Jiri
> On Nov 28, 2016, at 2:42 PM, Jiri Benc <jbenc@redhat.com> wrote: > > On Mon, 28 Nov 2016 14:29:39 -0800, Jarno Rajahalme wrote: >> I’m not sure what you suggest here. Obviously the kernel ABI can not >> be changed as existing userspace code expects upcalled packets to be >> non-accelerated. Also, if userspace pushes vlan headers, the packet >> will actually have them. > > The user space API needs to be preserved, of course. I'm talking about > what happens internally in the kernel. > > See this patchset: https://www.spinics.net/lists/netdev/msg398827.html > I did not try to apply this series yet, but given the recent L3 changes maybe it needs a rebase? >> Would this incremental fix this: >> >> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c >> index 9be9fda..37f1bb9 100644 >> --- a/net/openvswitch/flow.c >> +++ b/net/openvswitch/flow.c >> @@ -354,6 +354,8 @@ 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; >> + if (skb->protocol == htons(ETH_P_TEB)) >> + skb->protocol = key->eth.vlan.tpid; >> } >> >> /* Parse inner vlan tag. */ > > I'll look at this tomorrow. But it seems we're adding more and more > hacks instead of cleaning up the vlan handling. > Right, I just noticed that the incremental only handles the VLAN case. I’ll post a v2 later today with a proper fix that solves the immediate issue. IMO this should be fixed independently of the VLAN handling series for which I have no informed opinion yet. Jarno > Jiri
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 9be9fda..37f1bb9 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -354,6 +354,8 @@ 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; + if (skb->protocol == htons(ETH_P_TEB)) + skb->protocol = key->eth.vlan.tpid; } /* Parse inner vlan tag. */