[ovs-dev] [PATCH v2 3/3] ovn: Fix localnet ports on the same chassis.

Ben Pfaff blp at ovn.org
Thu Jan 21 22:35:11 UTC 2016


On Thu, Jan 21, 2016 at 03:30:50PM -0500, Russell Bryant wrote:
> On 01/20/2016 06:22 PM, Ben Pfaff wrote:
> > The new logic here in physical_run() looks wrong to me.  I think that it
> > strips a vlan if it didn't match on one.
> > 
> >             if (tag) {
> >                 match_set_dl_vlan(&match, htons(tag));
> >             } else if (!strcmp(binding->type, "localnet")) {
> >                 /* Match priority-tagged frames, e.g. VLAN ID 0.
> >                  *
> >                  * We'll add a second flow for frames that lack any 802.1Q
> >                  * header later. */
> >                 match_set_dl_tci_masked(&match, htons(VLAN_CFI),
> >                                         htons(VLAN_VID_MASK | VLAN_CFI));
> >             }
> > 
> >             /* Strip vlans. */
> >             if (tag) {
> >                 ofpact_put_STRIP_VLAN(&ofpacts);
> >             }
> > 
> > I believe that we can simplify it to the following, while fixing the bug
> > at the same time:
> > 
> >             /* Match a VLAN tag and strip it, including stripping priority tags
> >              * (e.g. VLAN ID 0).  In the latter case we'll add a second flow
> >              * for frames that lack any 802.1Q header later. */
> >             if (tag || !strcmp(binding->type, "localnet")) {
> >                 match_set_dl_vlan(&match, htons(tag));
> >                 ofpact_put_STRIP_VLAN(&ofpacts);
> >             }
> 
> This makes sense to me, as long as these two are functionally
> equivalent, because otherwise we'd be changing behavior from the current
> code.
> 
>     match_set_dl_vlan(&match, 0);
> 
>     match_set_dl_tci_masked(&match, htons(VLAN_CFI),
> 			    htons(VLAN_VID_MASK | VLAN_CFI));

They're equivalent; you can find that out, eventually, by looking at
what match_set_dl_vlan() does.



More information about the dev mailing list