On Wednesday, December 31, 2014 02:53:27 PM Thomas F Herbert wrote:
> Flavio,
>
> Thanks for your review. I am preparing a version 6 of the patch in which
> I will be fixing the issues you pointed out but I am interested in
> further guidance in your responses below.
>
> Thanks, Tom
>
>
> On 12/30/14, 3:30 PM, Flavio Leitner wrote:
> > On Tuesday, December 30, 2014 10:12:59 AM you wrote:
> >> This is the linux kernel datapath portion of the 802.1AD patch.
> >>
> >>
> >> Signed-off-by: Thomas F Herbert <thomasfherb...@entpnt.com>
> >>
> >> ---
> >> datapath/actions.c | 32 ++++++---
> >> datapath/flow.c | 80
> >> +++++++++++++++++++----
> >> datapath/flow.h | 1 +
> >> datapath/flow_netlink.c | 62 ++++++++++++++++--
> >> datapath/linux/compat/include/linux/openvswitch.h | 16 ++---
> >> 5 files changed, 153 insertions(+), 38 deletions(-)
> >>
> ...
> >>
> >>
> >> diff --git a/datapath/flow.c b/datapath/flow.c
> >> index 69b13b3..d549fae 100644
> >> --- a/datapath/flow.c
> >> +++ b/datapath/flow.c
> ...
> >> +
> >> + if (unlikely(skb->len < sizeof(struct qtag_prefix) +
> >> + sizeof(__be16)))
> >> + return 0;
> > This check is included in the pskb_may_pull() below.
> This puzzles me too. The double headroom check looks redundant but it is
> leveraged from the existing code that does it for 802.1q single vlans.
> Please advise, should we change this everywhere?
Interesting, so I went to git history and found this commit which
would explain the reason behind the extra check. It seems the
double checking is needed after all.
commit 8ddc056dd1e2c150c3bf8bb16811815736beb554
Author: Ben Pfaff <b...@nicira.com>
Date: Mon Nov 14 17:19:41 2011 -0800
datapath: Don't drop packets with partial vlan tags.
In the future it is likely that our vlan support will expand to
include multiply tagged packets. When this happens, we would
ideally like for it to be consistent with our current tagging.
Currently, if we receive a packet with a partial VLAN tag we will
automatically drop it in the kernel, which is unique among the
protocols we support. The only other reason to drop a packet is
a memory allocation error. For a doubly tagged packet, we will
parse the first tag and indicate that another tag was present but
do not drop if the second tag is incorrect as we do not parse it.
This changes the behavior of the vlan parser to match other protocols
and also deeper tags by indicating the presence of a broken tag with
the 802.1Q EtherType but no vlan information. This shifts the policy
decision to userspace on whether to drop broken tags and allows us to
uniformly add new levels of tag parsing.
Although additional levels of control are provided to userspace, this
maintains the current behavior of dropping packets with a broken
tag when using the NORMAL action because that is the correct behavior
for an 802.1Q-aware switch. The userspace flow parser actually
already had the new behavior so this corrects an inconsistency.
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
