Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR

[Ben Greear <greearb@candelatech.com>]

On 05/23/2011 12:36 PM, Nicolas de Pesloüan wrote:
> Le 23/05/2011 18:33, Ben Greear a écrit :
>> On 05/23/2011 02:00 AM, Eric W. Biederman wrote:
>>> Ben Greear<greearb@candelatech.com> writes:
>>>
>>>> I believe we have been getting tagged VLAN packets properly
>>>> in our test cases. We would not be creating any VLAN devices
>>>> in this case, so perhaps the NIC isn't doing any stripping.
>>>>
>>>> To me, it seems like we should get the fully tagged packet
>>>> without having to go muck with aux-data, though it would
>>>> be fine if it were *also* in aux-data.
>>>
>>> Given that pf_packet is a portable interface that works on multiple OS's
>>> I tend to agree. Certainly my users would be happier if they don't
>>> have to change their code and not having to change tcpdump would
>>> also be nice.
>>>
>>> I'm not certain exactly where in the code it makes sense to put the
>>> vlan header back on for pf_packet sockets. The simplest thing would
>>> be just before we run the socket filter. If we don't do the simplest
>>> thing this raises the question how do we avoid breaking socket filters
>>> that look at the packet data and know there is going to be a vlan
>>> header there.
>>
>> That is going to be tricky, since the VLAN header would adjust
>> offsets and users could be using some filter that uses offsets
>> with no actual mention of VLANs (but expecting it to take
>> the VLAN header into account).
>>
>>> Still the current situation is better than seeing vlan 0 tagged packets
>>> twice.
>>>
>>> My gut feel says if we can cheaply get the socket filters to act like it
>>> sees the vlan tag (where the vlan tag belongs) we should not actually
>>> put the vlan tag back on until we copy the packet to userspace.
>>
>> Maybe keep a count of how many sockets with filters and/or pf_packet
>> sockets are open, and how many things are registered in
>> the 'ptype_all' chain, and only re-add (or never remove) the header if
>> that is > 0?
>>
>> (And, let the bridging and other kernel logic deal with vlans
>> via auxillary methods as well as checking in-line headers.)
>
> Well, this doesn't sound very different from my previous proposal: if a
> protocol handler is registered at parent interface level, can't we
> simply assume this protocol handler expect the raw packet?

Well, yes..unless perhaps when the REORDER_HDR flag is enabled.

As for when the tag is added/removed, as long as we don't end up doing
a remove and then an add (in software), and as long as the pkt is correct
going to user-space, it doesn't matter to me.

It would be lame to remove it in software and then re-add it,
unless absolutely required for some reason.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com