On Mon, Oct 03, 2016 at 01:16:55PM -0700, Ben Pfaff wrote:
> On Fri, Sep 23, 2016 at 03:15:27PM -0400, Eric Garver wrote:
> > We need to check if a packet is double tagged. If so make sure to push
> > 0x88a8 instead of 0x8100. Without this a simple port redirect of 802.1ad
> > frames means the outer tag gets translated from 0x88a8 to 0x8100 by the
> > userspace datapath.
> > 
> > This only affected kernels that don't use TP_STATUS_VLAN_TPID_VALID,
> > which is kernels < 3.14.
> > 
> > Signed-off-by: Eric Garver <e...@erig.me>
> 
> ...
> 
> >          if (auxdata_has_vlan_tci(aux)) {
> > +            struct eth_header *eth = dp_packet_data(buffer);
> > +            bool doubleTagged = eth->eth_type == 
> > htons(ETH_TYPE_VLAN_8021Q);
> > +
> >              if (retval < ETH_HEADER_LEN) {
> >                  return EINVAL;
> >              }
> >  
> > -            eth_push_vlan(buffer, auxdata_to_vlan_tpid(aux),
> > +            eth_push_vlan(buffer, auxdata_to_vlan_tpid(aux, doubleTagged),
> >                            htons(aux->tp_vlan_tci));
> >              break;
> >          }
> 
> The new code here dereferences eth->eth_type before checking that the
> packet is long enough.  That is, the retval check should precede the
> doubletagged check.
> 

Oops. Thanks for catching.
Will send a v2.

> As a minor point of style, we prefer underscores over capitals:
> s/doubleTagged/double_tagged/.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to