On Tue, Dec 19, 2017 at 03:42:47PM -0500, Eric Garver wrote:
> On Tue, Dec 19, 2017 at 08:39:29PM +0100, Jiri Benc wrote:
> > On Tue, 19 Dec 2017 13:57:53 -0500, Eric Garver wrote:
> > > --- a/net/openvswitch/flow.c
> > > +++ b/net/openvswitch/flow.c
> > > @@ -559,8 +559,9 @@ static int parse_nsh(struct sk_buff *skb, struct 
> > > sw_flow_key *key)
> > >   *      of a correct length, otherwise the same as skb->network_header.
> > >   *      For other key->eth.type values it is left untouched.
> > >   *
> > > - *    - skb->protocol: the type of the data starting at 
> > > skb->network_header.
> > > - *      Equals to key->eth.type.
> > > + *    - skb->protocol: For Ethernet, the ethertype or VLAN TPID.
> > > + *      For non-Ethernet, the type of the data starting at 
> > > skb->network_header
> > > + *      (also equal to key->eth.type).
> > >   */
> > >  static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
> > >  {
> > > @@ -579,6 +580,7 @@ static int key_extract(struct sk_buff *skb, struct 
> > > sw_flow_key *key)
> > >                   return -EINVAL;
> > >  
> > >           skb_reset_network_header(skb);
> > > +         key->eth.type = skb->protocol;
> > >   } else {
> > >           eth = eth_hdr(skb);
> > >           ether_addr_copy(key->eth.src, eth->h_source);
> > > @@ -592,15 +594,14 @@ static int key_extract(struct sk_buff *skb, struct 
> > > sw_flow_key *key)
> > >           if (unlikely(parse_vlan(skb, key)))
> > >                   return -ENOMEM;
> > >  
> > > -         skb->protocol = parse_ethertype(skb);
> > > -         if (unlikely(skb->protocol == htons(0)))
> > > +         key->eth.type = parse_ethertype(skb);
> > > +         if (unlikely(key->eth.type == htons(0)))
> > >                   return -ENOMEM;
> > >  
> > >           skb_reset_network_header(skb);
> > >           __skb_push(skb, skb->data - skb_mac_header(skb));
> > >   }
> > >   skb_reset_mac_len(skb);
> > > - key->eth.type = skb->protocol;
> > >  
> > >   /* Network layer. */
> > >   if (key->eth.type == htons(ETH_P_IP)) {
> > 
> > Unfortunately, this does not work. key_extract must set skb->protocol
> > even for Ethernet frames that come from a mixed L2/L3 tunnel. Such
> > packets will have key->mac_proto set to MAC_PROTO_ETHERNET and
> > skb->protocol set to ETH_P_TEB (see key_extract_mac_proto). In
> > key_extract, skb->protocol has to be correctly set to the dissected
> > value.
> 
> AFAICS, it's always overridden to ETH_P_TEB on output by
> ovs_vport_send() and that's the sole reason it works today.
> 
> For dissecting, the L2 case is currently setting skb->protocol to the
> real ethertype (e.g. 0x800) not ETH_P_TEB. For RX from tunnel case it'll
> indeed be ETH_P_TEB.

Of course after I hit send I realize what you were saying. I follow now
why skb->protocol needs to be the real ethertype.

> > 
> > Which means that we have to check for the existence of inner vlan tag
> > (by checking key->eth.cvlan.tci or, perhaps better, by returning it
> > from parse_vlan) and set skb->protocol accordingly.

I'll see what we can do here. Thanks Jiri.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to