[ovs-dev] [PATCH] datapath: Don't drop packets with partial vlan tags.

[Ben Pfaff]

On Mon, Nov 14, 2011 at 05:06:39PM -0800, Jesse Gross wrote:
> On Mon, Nov 14, 2011 at 4:32 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 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.
> >
> > [Original patch and commit message by Jesse Gross.]
> 
> If you think this is now ready, can you add a signed-off-by?

Oops.  Will do.

I'll resend.

> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index b974dcc..2edba2d 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -622,7 +632,8 @@ parse_odp_key_attr(const char *s, struct ofpbuf *key)
> > ?? ?? ?? ?? ?? ?? ??&& n > 0)) {
> > ?? ?? ?? ?? ?? ?? nl_msg_put_be16(key, OVS_KEY_ATTR_VLAN,
> > ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? htons((vid << VLAN_VID_SHIFT) |
> > - ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??(pcp << VLAN_PCP_SHIFT)));
> > + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??(pcp << VLAN_PCP_SHIFT) |
> > + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??VLAN_CFI));
> 
> I realized that we don't parse "cfi" for broken vlan tags here.

OK, this at least compiles:

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 2edba2d..3821553 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -626,6 +626,7 @@ parse_odp_key_attr(const char *s, struct ofpbuf
*key)
     {
         uint16_t vid;
         int pcp;
+        int cfi;
         int n = -1;
 
         if ((sscanf(s, "vlan(vid=%"SCNi16",pcp=%i)%n", &vid, &pcp,
         &n) > 0
@@ -635,6 +636,14 @@ parse_odp_key_attr(const char *s, struct ofpbuf
         *key)
                                   (pcp << VLAN_PCP_SHIFT) |
                                   VLAN_CFI));
             return n;
+        } else if ((sscanf(s, "vlan(vid=%"SCNi16",pcp=%i,cfi=%i)%n",
+                           &vid, &pcp, &cfi, &n) > 0
+             && n > 0)) {
+            nl_msg_put_be16(key, OVS_KEY_ATTR_VLAN,
+                            htons((vid << VLAN_VID_SHIFT) |
+                                  (pcp << VLAN_PCP_SHIFT) |
+                                  (cfi ? VLAN_CFI : 0)));
+            return n;
         }
     }