On Wed, May 11, 2016 at 04:09:28PM +0200, Jiri Benc wrote:
> On Wed, 11 May 2016 12:06:35 +0900, Simon Horman wrote:
> > Is this close to what you had in mind?
>
> Yes but see below.
>
> > @@ -739,17 +729,17 @@ int ovs_flow_key_extract(const struct ip_tunnel_info
> > *tun_info,
> > key->phy.skb_mark = skb->mark;
> > ovs_ct_fill_key(skb, key);
> > key->ovs_flow_hash = 0;
> > - key->phy.is_layer3 = is_layer3;
> > + key->phy.is_layer3 = (tun_info && skb->mac_len == 0);
>
> Do we have to depend on tun_info? It would be nice to support all
> ARPHRD_NONE interfaces, not just tunnels. The tun interface (from
> the tuntap driver) comes to mind, for example.
Yes, I think that should work. I was just being cautious.
Do you think it is safe to detect TEB based on skb->protocol regardless
of the presence of tun_info?
> > +++ b/net/openvswitch/vport-netdev.c
> > @@ -60,7 +60,21 @@ static void netdev_port_receive(struct sk_buff *skb)
> > if (vport->dev->type == ARPHRD_ETHER) {
> > skb_push(skb, ETH_HLEN);
> > skb_postpush_rcsum(skb, skb->data, ETH_HLEN);
> > + } else if (vport->dev->type == ARPHRD_NONE) {
> > + if (skb->protocol == htons(ETH_P_TEB)) {
> > + struct ethhdr *eth = eth_hdr(skb);
> > +
> > + if (unlikely(skb->len < ETH_HLEN))
> > + goto error;
> > +
> > + skb->mac_len = ETH_HLEN;
> > + if (eth->h_proto == htons(ETH_P_8021Q))
> > + skb->mac_len += VLAN_HLEN;
> > + } else {
> > + skb->mac_len = 0;
> > + }
>
> Without putting much thought into this, could this perhaps be left for
> parse_ethertype (called from key_extract) to do?
I think I am confused.
I believe that key_extract() does already do all of the above (and more).
The purpose of the above change was to do this work here rather than
leaving it to parse_ethertype. This is because I was under the impression
that is what you were after. Specifically as a mechanism to avoid relying
on vport->dev->type in ovs_flow_key_extract.
If we can live with a bogus skb->mac_len value that is sufficient for
ovs_flow_key_extract.() and set correctly by key_extract() (which happens
anyway) we could do something like this:
} else if (vport->dev->type == ARPHRD_NONE) {
if (skb->protocol == htons(ETH_P_TEB))
/* Ignores presence of VLAN but is sufficient for
* ovs_flow_key_extract() which then calls key_extract()
* which calculates skb->mac_len correctly. */
skb->mac_len = ETH_HLEN; /* Ignores presence of VLAN */
else
skb->mac_len = 0;
}
But perhaps I have missed the point somehow.
