On Tue, May 17, 2016 at 04:43:20PM +0200, Jiri Benc wrote: > On Thu, 12 May 2016 07:46:52 +0900, Simon Horman wrote: > > 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. > > You did not, I was more thinking aloud. But you're right, it doesn't > make much sense. > > So, wouldn't it be actually more correct and in line with patch 2 to > call eth_type_trans() here?
Yes, that seems reasonable. > Possibly even followed by skb_vlan_untag > (not needed. But it might make things easier to have the first vlan tag > always reside inside skb->vlan_tci and treat that as an invariant in > the whole ovs kernel code. It'll need to be done in more spots than > just this one, though, and is probably matter of a separate patchset). That also seems reasonable. But as it seems more invasive and is not strictly necessary I'd rather handle that update separately.