Message-ID: <CAD6jFUTht82HOjGjDU7hFCEWyE3TOx_W4_j=SZK-DrcGfrio-A@mail.gmail.com>
Date:	Thu, 13 Dec 2012 22:49:38 +0100
From:	Daniel Borkmann <danborkmann@...earbox.net>
To:	Ani Sinha <ani@...stanetworks.com>
Cc:	Michael Richardson <mcr@...delman.ca>, netdev@...r.kernel.org,
	tcpdump-workers@...ts.tcpdump.org,
	Francesco Ruggeri <fruggeri@...stanetworks.com>
Subject: Re: [tcpdump-workers] vlan tagged packets and libpcap breakage

On Thu, Dec 13, 2012 at 6:34 PM, Ani Sinha <ani@...stanetworks.com> wrote:
> On Thu, Dec 13, 2012 at 12:35 AM, Daniel Borkmann <dborkman@...hat.com> wrote:
>> On 12/12/2012 10:53 PM, Ani Sinha wrote:
>>>>
>>>> unsigned int netdev_8021q_inskb = 1;
>>>>
>>>> ...
>>>>         {
>>>>                 .ctl_name       = NET_CORE_8021q_INSKB,
>>>>                 .procname       = "netdev_8021q_inskb",
>>>>                 .data           = &netdev_8021q_inskb,
>>>>                 .maxlen         = sizeof(int),
>>>>                 .mode           = 0444,
>>>>                 .proc_handler   = proc_dointvec
>>>>         },
>>>>
>>>> would seem to do it to me.
>>>> Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
>>>> finds it, and it is >0, then do the cmsg thing.
>>>>
>>
>> I think it doesn't. Because then you are obviously considering adding one
>> procfs file into /proc/sys/net/core/ *for each* feature that is added into
>> the ancillary ops which cannot be the right way ...
>
> We had already brought up this topic previously in the same thread. A
> suggestion was made to add that proc entry and no one from netdev
> responded to it saying that it did not make any sense. Therefore
> before I went ahead and made the fixes in libpcap, I wanted to run
> this by your guys again to make sure we are still on the same page.

Well, not everyone on netdev might be following this thread (resp.
following fully). The best way to get responses for a patch is to go
through the normal patch submission process on netdev, and if you like
to request for comments, then mark it as RFC in the subject. This way,
people will know and likely comment on it if it makes sense or not.

> I do agree that instead of a /proc entry, we should check for a kenrel
> version >= X where X is the upstream version that first started
> supporting all the features needed by libpcap for vlan filtering. This
> is not a compile time check but a run time one. Does anyone see any
> issues with this? Is there any long term implications of this, like if
> you backport patches to an older long term supported kernel? Are there
> other better ways to do this, like may be returning feature bits from
> an ioctl call? This is something we need to deal with on a continuous
> basis as we keep supporting newer AUX fields and libpcap and other
> user land code needs to make use of it. At the same time, they need to
> handle backward compatibility issues with older kernels.

As Eric mentioned earlier, for now there seems not to be a reliable
way to get to know which ops are present and which not. It's not
really nice, but if you want to make use of those new (ANC*) features,
probably checking kernel version might be the only way if I'm not
missing something. Now net-next is closed, but if it reopens, I'll
submit a version 2 of my patch where you've been CC'd to. If it gets
in, then at least it's for sure that since kernel <xyz> this kind of
feature test is present.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists