Re: [Bridge] [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in separate functions.

[Vlad Yasevich <vyasevic@redhat.com>]

On 01/11/2013 01:23 PM, Stephen Hemminger wrote:
> On Fri, 11 Jan 2013 12:41:48 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 01/11/2013 12:20 PM, Stephen Hemminger wrote:
>>> What I think is the least intrusive and allows for maximum flexibility
>>> is having the VLAN bridge filtering depend on VLAN support (CONFIG_VLAN_8021Q).
>>> There already is drivers that depend on that value to enable filtering.
>>>
>>
>> The only thing that I see depending on CONFIG_VLAN_8021Q is
>> CONFIG_VLAN_8021Q_GVRP which is part of the 8021Q support.
>> There are currently no other drivers depending on 8021Q functionality
>> and vlan filtering in drivers doesn't depend on 8021Q support in
>> the kernel.
>>
>> I admit that I've thought of having a dependency on 8021Q as it would
>> have allowed me to re-use a bit more code, but decided that bridge
>> should be able to stand on its own in this regard.  8021Q is not
>> necessary to turn on VLAN accelerated filtering on the nics as anyone
>> can do it through the ndo_vlan_rx_add_vid() call.
>>
>> The reason for this patch was to make the nic vlan filter code reusable
>> and address Jiri Pirko's comment in the V2 series.
>> (http://marc.info/?l=linux-netdev&m=135590565719164&w=2).  This way,
>> bridge wouldn't need to make direct ndo_ calls and all call sights will
>> be consistent.
>>
>>> And make the support of VLAN filtering in the bridge conditional like
>>> IGMP snooping is optional
>>
>> I could certainly make the VLAN filtering conditional, but I am not sure
>> what it would buy us other then a lot of ifdefs.
>>
>> Thanks
>> -vlad
>>
>>>
>>> --- a/net/bridge/Kconfig
>>> +++ b/net/bridge/Kconfig
>>> @@ -46,3 +46,17 @@ config BRIDGE_IGMP_SNOOPING
>>>             Say N to exclude this support and reduce the binary size.
>>>
>>>             If unsure, say Y.
>>> +
>>> +config BRIDGE_VLAN_FILTERING
>>> +       bool "VLAN filtering"
>>> +       depends on BRIDGE
>>> +       depends on VLAN_8021Q
>>> +       default n
>>> +       ---help---
>>> +         If you say Y here, then the Ethernet bridge will be able to
>>> +        selectively filter traffic based on VLAN tag.
>>> +
>>> +        Say N to exclude this support and reduce the binary size.
>>> +
>>> +        If unsure, say Y.
>>>
>>
>
> Doing the following shows several drivers that still depend on VLAN_8021Q being
> enabled to do VLAN tagging.
> $ git grep -l CONFIG_VLAN_8021Q
> ethernet/3com/3c59x.c
> ethernet/adaptec/starfire.c
> ethernet/amd/amd8111e.c
> ethernet/broadcom/cnic.c
> ethernet/broadcom/tg3.c
> ethernet/dlink/sundance.c
> ethernet/natsemi/ns83820.c
> ethernet/sis/sis900.c
> ethernet/sis/sis900.h
> ethernet/stmicro/stmmac/common.h


OK.  You've convinced me.  I can make my code depend on 8021Q and move 
these functions into vlan module.
I'll update the series with these changes.

-vlad