Re: [patch net-next-2.6 v2] net: vlan: make non-hw-accel rx path similar to hw-accel

[Jiri Pirko <jpirko@redhat.com>]

Sun, May 22, 2011 at 11:20:09AM CEST, mirqus@gmail.com wrote:
>W dniu 22 maja 2011 11:10 użytkownik Nicolas de Pesloüan
><nicolas.2p.debian@gmail.com> napisał:
>> Le 22/05/2011 10:52, Michał Mirosław a écrit :
>>>
>>> 2011/5/22 Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>:
>>>>
>>>> Le 22/05/2011 08:34, Eric W. Biederman a écrit :
>>>>>
>>>>> Jiri Pirko<jpirko@redhat.com>    writes:
>>>>>
>>>>>> Sun, May 22, 2011 at 04:59:49AM CEST, nicolas.2p.debian@gmail.com
>>>>>> wrote:
>>>>>>
>>>>>> <snip>
>>>>>>>
>>>>>>> And because some setups may still require the skb not to be untagged,
>>>>>>> may be we need the ability to re-tag the skb in some situations...
>>>>>>> When a protocol handler or rx_handler is explicitly registered on a
>>>>>>> net_device which expect to receive tagged skb, we should deliver
>>>>>>> tagged skb to it... Arguably, this may sound incredible for the
>>>>>>> general case, but may be required for not-so-special cases like
>>>>>>> bridge or protocol analyzer.
>>>>>>
>>>>>> Wait, what setups/code require the skb not to be untagged? If there's
>>>>>> such, it should be fixed.
>>>>>
>>>>> tcpdump on the non-vlan interface for one.
>>>>
>>>> bridge is another. More precisely, there is a difference between the
>>>> following two setups:
>>>>
>>>> 1/ eth0 - eth0.100 - br0 - eth1.200 - eth1
>>>>
>>>> 2/ eth0 - br0 - eth1
>>>>
>>>> In case 1, it is normal and desirable for the bridge to see untagged skb.
>>>>
>>>> In case 2, it is desirable for the bridge to see untouched (possibly
>>>> tagged)
>>>> skb. If current bridge implementation is able to handle skb from which we
>>>> removed a tag, in this situation, it means that bridge currently "fix
>>>> improper untagging" by itself, by forcing re-tagging on output. I think
>>>> is
>>>> should not be the job of protocol handlers to fix this. Again, a generic
>>>> feature should to it when necessary.
>>>>
>>>> Think of the following setups:
>>>>
>>>> 3/ eth0 - br0 - eth1.200 - eth1.
>>>> 4/ eth0 - eth0.100 - br0 - eth1
>>>>
>>>> What if one expect this setup to add (3) or remove (4) one level of vlan
>>>> nesting? This is precisely what this setup suggest. How can we instruct
>>>> the
>>>> bridge to do so? It is not the bridge responsibility to do any vlan
>>>> processing. bridge is expected to... bridge !
>>>
>>> I assumed that this untaging Jiri is implementing does not remove the
>>> tag. It moves the information from skb->data to skb->vlan_tci, but the
>>> information contained is not otherwise changing. All your examples
>>> should work regardless of where the tag is stored.
>>
>> I assumed (but didn't tested) that this untagging also change the starting
>> point of the payload of the packet. So protocol handlers expecting to have
>> the raw packet won't see the vlan header.
>
>That would also be the case with hardware stripped tags - they need to
>look into skb->vlan_tci anyway.

Exactly. Nicolas, I do not see anything wrong on always untagging in all
your setups. As Michal said, vlan_tci keeps the info.

Jirka

>
>Best Regards,
>Michał Mirosław