diff mbox

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

Message ID 20130111092004.2d55ae65@nehalam.linuxnetplumber.net
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

stephen hemminger Jan. 11, 2013, 5:20 p.m. UTC 
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.

And make the support of VLAN filtering in the bridge conditional like
IGMP snooping is optional

the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Vlad Yasevich Jan. 11, 2013, 5:41 p.m. UTC | #1 
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.
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger Jan. 11, 2013, 6:23 p.m. UTC | #2 
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

 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Yasevich Jan. 11, 2013, 6:53 p.m. UTC | #3 
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in