On Mon, Nov 14, 2011 at 05:06:39PM -0800, Jesse Gross wrote: > On Mon, Nov 14, 2011 at 4:32 PM, Ben Pfaff <b...@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; } } _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev