Re: [patch net-next v4 8/9] net: move vlan pop/push functions into common code

[Jiri Benc <jbenc@redhat.com>]

On Fri, 21 Nov 2014 10:41:27 -0800, Pravin Shelar wrote:
> There is bug in the openvswitch code where vlan-pop does mac-length
> reset rather than subtracting VLAN_HLEN  from mac-len. MPLS code
> depends on this. Now this patch moves the bug to common code. I am
> fine with this patch and I can send fix to change code vlan-pop code
> to adjust mac_len later on.
> But I am worried about the comment made by Jiri Benc on earlier
> version of this patch where is mentioned that subtracting VLAN_HLEN
> can break common case for vlan-pop. In that case we can not change
> this common function. Jiri Benc, Can you point me to the code, so the
> we can work on solution to fix this issue.

In such case, a skb that enters the __pop_vlan_tci function looks like
this:

+--------+--------+-------------+-----+---------+-------
| dstmac | srcmac | ETH_P_8021Q | tci | ETH_P_x | data
+--------+--------+-------------+-----+---------+-------
^                               ^
mac_header                      network_header

skb->protocol = ETH_P_8021Q
skb->mac_len = 14

After the function finishes, it's supposed to look like this:

                    +--------+--------+---------+-------
                    | dstmac | srcmac | ETH_P_x | data
                    +--------+--------+---------+-------
                    ^                           ^
                    mac_header                  network_header

skb->protocol = ETH_P_x
skb->mac_len = 14

Blindly decrementing mac_len wouldn't work here, you'd end up with
mac_len = 10. You'd either need to ensure this case doesn't happen (it
does with openvswitch currently) or detect this case.

 Jiri

-- 
Jiri Benc