Thanks.  I'm sure it needs a rebase, that's true pretty much all the
time ;-)

On Tue, Jul 28, 2015 at 04:50:54PM -0700, Gurucharan Shetty wrote:
> Okay. (I already committed this patch, so you probably need a rebase)
> 
> On Tue, Jul 28, 2015 at 3:42 PM, Ben Pfaff <b...@nicira.com> wrote:
> > On Tue, Jul 28, 2015 at 01:44:42PM -0700, Ben Pfaff wrote:
> >> On Fri, Jul 24, 2015 at 04:31:03PM -0700, Gurucharan Shetty wrote:
> >> > In table 64, when a vlan tag is set on a packet
> >> > destined to a container running inside a VM, we currently
> >> > don't revert it. This has an unintended consequence for
> >> > broadcast traffic when one endpoint of the braodcast
> >> > traffic is a plain VM (without containers running inside) where
> >> > the previously set tag would remain in the packets sent to the VM.
> >> >
> >> > This commit fixes the above problem by popping the VLAN
> >> > and resetting the input port after outputting the packet
> >> > with a vlan tag to a container logical port.
> >> >
> >> > Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
> >>
> >> Acked-by: Ben Pfaff <b...@nicira.com>
> >
> > The same bug is in my tunnel-key series, I'm folding the following into
> > the final patch in that series.
> >
> > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> > index 09b7a99..4c81bb2 100644
> > --- a/ovn/controller/physical.c
> > +++ b/ovn/controller/physical.c
> > @@ -123,6 +123,14 @@ put_encapsulation(enum mf_field_id mff_ovn_geneve,
> >      }
> >  }
> >
> > +static void
> > +put_stack(enum mf_field_id field, struct ofpact_stack *stack)
> > +{
> > +    stack->subfield.field = mf_from_id(field);
> > +    stack->subfield.ofs = 0;
> > +    stack->subfield.n_bits = stack->subfield.field->n_bits;
> > +}
> > +
> >  void
> >  physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
> >               const struct ovsrec_bridge *br_int, const char 
> > *this_chassis_id,
> > @@ -301,9 +309,19 @@ physical_run(struct controller_ctx *ctx, enum 
> > mf_field_id mff_ovn_geneve,
> >                  /* A packet might need to hair-pin back into its ingress
> >                   * OpenFlow port (to a different logical port, which we 
> > already
> >                   * checked back in table 34), so set the in_port to zero. 
> > */
> > +                put_stack(MFF_IN_PORT, ofpact_put_STACK_PUSH(&ofpacts));
> >                  put_load(0, MFF_IN_PORT, 0, 16, &ofpacts);
> >              }
> >              ofpact_put_OUTPUT(&ofpacts)->port = ofport;
> > +            if (tag) {
> > +                /* Revert the tag added to the packets headed to containers
> > +                 * in the previous step. If we don't do this, the packets
> > +                 * that are to be broadcasted to a VM in the same logical
> > +                 * switch will also contain the tag. Also revert the zero'd
> > +                 * in_port. */
> > +                ofpact_put_STRIP_VLAN(&ofpacts);
> > +                put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(&ofpacts));
> > +            }
> >              ofctrl_add_flow(flow_table, 64, 100, &match, &ofpacts);
> >          } else {
> >              /* Table 32, priority 100.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to