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? 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).
Jiri
