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

[Ben Greear <greearb@candelatech.com>]

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

Thanks,
Ben


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