diff mbox series

[OpenWrt-Devel,PATCHv2] Allow VLAN filtering if needed.

Message ID d8ac6b39-78ca-6a7f-e05b-1d7d6e59c184@navigue.com
State Accepted
Headers show
Series [OpenWrt-Devel,PATCHv2] Allow VLAN filtering if needed. | expand

Commit Message

Jonathan Thibault Nov. 15, 2018, 4:12 p.m. UTC
Greetings,

I would like to propose enabling CONFIG_BRIDGE_VLAN_FILTERING on OpenWRT 
releases.

This allows us to use the bridge as a managed switch and gracefully 
handle mixed tagged and untagged frames. Prior to this, the only 
alternative was creating one bridge per vlan which quickly becomes a 
nightmare and still won't let you mix both tagged and untagged frames on 
the physical port without some complex ebtables magic.

This is in line with the notion that OpenWRT is the network go-to swiss 
army knife when you need a nice set-and-forget, low maintenance box to 
handle a specific task.

Current builds of the ip-bridge package already fully support this 
feature so the only requirement is enabling the kernel config.

This is disabled by default so existing bridge configurations will not 
be affected.  This patch only gives the ability to turn it on with an 
'ip link' command.  If there is interest, I could look into making the 
feature accessible via uci configuration.

It causes about 3.1% hit on raw bridging speed, which is relatively 
trivial considering that I had to use 300 byte packets to strain the CPU 
enough to notice a slowdown at all.  The ER8 would chug along at wire 
speed otherwise, and that's using only one core.  Since the typical 
bridge use case on OpenWRT is wireless, I doubt it would be noticeable 
at all.

With BRIDGE_VLAN_FILTERING

iperf -u -c 192.168.1.105 -b 1G -l 300
------------------------------------------------------------
Client connecting to 192.168.1.105, UDP port 5001
Sending 300 byte datagrams, IPG target: 2.24 us (kalman adjust)
UDP buffer size:  208 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.12 port 58045 connected with 192.168.1.105 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.0 sec   977 MBytes   820 Mbits/sec
[  3] Sent 3414986 datagrams
[  3] Server Report:
[  3]  0.0-10.0 sec   811 MBytes   680 Mbits/sec   0.000 ms 
581210/3414986 (0%)

Without BRIDGE_VLAN_FILTERING

iperf -u -c 192.168.1.105 -b 1G -l 300
------------------------------------------------------------
Client connecting to 192.168.1.105, UDP port 5001
Sending 300 byte datagrams, IPG target: 2.24 us (kalman adjust)
UDP buffer size:  208 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.12 port 36645 connected with 192.168.1.105 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.0 sec   977 MBytes   820 Mbits/sec
[  3] Sent 3414990 datagrams
[  3] Server Report:
[  3]  0.0-10.0 sec   836 MBytes   701 Mbits/sec   0.000 ms 
493950/3414990 (0%)

In terms of kernel size, it uses 16KB (6753K vs 6737K on ER8) so a 
0.002% hit.  The exact 16KB is probably just due to how the kernel is 
compressed.

---

v2: Enable the right CONFIG_ on 4.14.  Thanks Felix for pointing out my 
silliness :)

Comments

Daniel Golle Nov. 15, 2018, 6:58 p.m. UTC | #1
Hi Jonathan,

On Thu, Nov 15, 2018 at 11:12:54AM -0500, Jonathan Thibault wrote:
> Greetings,
> 
> I would like to propose enabling CONFIG_BRIDGE_VLAN_FILTERING on OpenWRT
> releases.

I've suggested that already about a year ago. Now that a significant
amount of targets uses DSA this is even more needed and I think we
should enable it now before the 2019 release is coming up.

> 
> This allows us to use the bridge as a managed switch and gracefully handle
> mixed tagged and untagged frames. Prior to this, the only alternative was
> creating one bridge per vlan which quickly becomes a nightmare and still
> won't let you mix both tagged and untagged frames on the physical port
> without some complex ebtables magic.
> 
> This is in line with the notion that OpenWRT is the network go-to swiss army
> knife when you need a nice set-and-forget, low maintenance box to handle a
> specific task.
> 
> Current builds of the ip-bridge package already fully support this feature
> so the only requirement is enabling the kernel config.
> 
> This is disabled by default so existing bridge configurations will not be
> affected.  This patch only gives the ability to turn it on with an 'ip link'
> command.  If there is interest, I could look into making the feature
> accessible via uci configuration.
> 
> It causes about 3.1% hit on raw bridging speed, which is relatively trivial
> considering that I had to use 300 byte packets to strain the CPU enough to
> notice a slowdown at all.  The ER8 would chug along at wire speed otherwise,
> and that's using only one core.  Since the typical bridge use case on
> OpenWRT is wireless, I doubt it would be noticeable at all.
> 
> With BRIDGE_VLAN_FILTERING
> 
> iperf -u -c 192.168.1.105 -b 1G -l 300
> ------------------------------------------------------------
> Client connecting to 192.168.1.105, UDP port 5001
> Sending 300 byte datagrams, IPG target: 2.24 us (kalman adjust)
> UDP buffer size:  208 KByte (default)
> ------------------------------------------------------------
> [  3] local 192.168.1.12 port 58045 connected with 192.168.1.105 port 5001
> [ ID] Interval       Transfer     Bandwidth
> [  3]  0.0-10.0 sec   977 MBytes   820 Mbits/sec
> [  3] Sent 3414986 datagrams
> [  3] Server Report:
> [  3]  0.0-10.0 sec   811 MBytes   680 Mbits/sec   0.000 ms 581210/3414986
> (0%)
> 
> Without BRIDGE_VLAN_FILTERING
> 
> iperf -u -c 192.168.1.105 -b 1G -l 300
> ------------------------------------------------------------
> Client connecting to 192.168.1.105, UDP port 5001
> Sending 300 byte datagrams, IPG target: 2.24 us (kalman adjust)
> UDP buffer size:  208 KByte (default)
> ------------------------------------------------------------
> [  3] local 192.168.1.12 port 36645 connected with 192.168.1.105 port 5001
> [ ID] Interval       Transfer     Bandwidth
> [  3]  0.0-10.0 sec   977 MBytes   820 Mbits/sec
> [  3] Sent 3414990 datagrams
> [  3] Server Report:
> [  3]  0.0-10.0 sec   836 MBytes   701 Mbits/sec   0.000 ms 493950/3414990
> (0%)
> 
> In terms of kernel size, it uses 16KB (6753K vs 6737K on ER8) so a 0.002%
> hit.  The exact 16KB is probably just due to how the kernel is compressed.
> 
> ---
> 
> v2: Enable the right CONFIG_ on 4.14.  Thanks Felix for pointing out my
> silliness :)
> 
> diff --git a/target/linux/generic/config-4.14
> b/target/linux/generic/config-4.14
> index 97207cf2eb..7d4750f461 100644
> --- a/target/linux/generic/config-4.14
> +++ b/target/linux/generic/config-4.14
> @@ -627,7 +627,7 @@ CONFIG_BRIDGE=y
>  CONFIG_BRIDGE_IGMP_SNOOPING=y
>  # CONFIG_BRIDGE_NETFILTER is not set
>  # CONFIG_BRIDGE_NF_EBTABLES is not set
> -# CONFIG_BRIDGE_VLAN_FILTERING is not set
> +CONFIG_BRIDGE_VLAN_FILTERING=y
>  # CONFIG_BROADCOM_PHY is not set
>  CONFIG_BROKEN_ON_SMP=y
>  # CONFIG_BSD_DISKLABEL is not set
> diff --git a/target/linux/generic/config-4.9
> b/target/linux/generic/config-4.9
> index 979028f04a..860b39b428 100644
> --- a/target/linux/generic/config-4.9
> +++ b/target/linux/generic/config-4.9
> @@ -598,7 +598,7 @@ CONFIG_BRIDGE=y
>  CONFIG_BRIDGE_IGMP_SNOOPING=y
>  # CONFIG_BRIDGE_NETFILTER is not set
>  # CONFIG_BRIDGE_NF_EBTABLES is not set
> -# CONFIG_BRIDGE_VLAN_FILTERING is not set
> +CONFIG_BRIDGE_VLAN_FILTERING=y
>  # CONFIG_BROADCOM_PHY is not set
>  CONFIG_BROKEN_ON_SMP=y
>  # CONFIG_BSD_DISKLABEL is not set
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Daniel Golle Nov. 21, 2018, 2:13 p.m. UTC | #2
Hi!

On Thu, Nov 15, 2018 at 07:58:07PM +0100, Daniel Golle wrote:
> Hi Jonathan,
> 
> On Thu, Nov 15, 2018 at 11:12:54AM -0500, Jonathan Thibault wrote:
> > Greetings,
> > 
> > I would like to propose enabling CONFIG_BRIDGE_VLAN_FILTERING on OpenWRT
> > releases.
> 
> I've suggested that already about a year ago. Now that a significant
> amount of targets uses DSA this is even more needed and I think we
> should enable it now before the 2019 release is coming up.

I'm about to merge this into master with my SoB added as you didn't
sign off the patch (which is so trivial that in practise this doesn't
matter). Find it in my staging tree [1] and raise your voice in case
of any concerns. Otherwise I'll merge it by the end of the week.


Cheers

Daniel

[1]: https://git.openwrt.org/?p=openwrt/staging/dangole.git
Jonathan Thibault Nov. 21, 2018, 4:01 p.m. UTC | #3
On 21/11/18 09:13 AM, Daniel Golle wrote:
> Hi!
>
> On Thu, Nov 15, 2018 at 07:58:07PM +0100, Daniel Golle wrote:
>> Hi Jonathan,
>>
>> On Thu, Nov 15, 2018 at 11:12:54AM -0500, Jonathan Thibault wrote:
>>> Greetings,
>>>
>>> I would like to propose enabling CONFIG_BRIDGE_VLAN_FILTERING on OpenWRT
>>> releases.
>> I've suggested that already about a year ago. Now that a significant
>> amount of targets uses DSA this is even more needed and I think we
>> should enable it now before the 2019 release is coming up.
> I'm about to merge this into master with my SoB added as you didn't
> sign off the patch (which is so trivial that in practise this doesn't
> matter). Find it in my staging tree [1] and raise your voice in case
> of any concerns. Otherwise I'll merge it by the end of the week.
That's perfectly fine.  I'm just happy it's getting merged, OpenWRT just 
got more useful.
Hauke Mehrtens Nov. 21, 2018, 7:55 p.m. UTC | #4
On 11/21/18 5:01 PM, Jonathan Thibault wrote:
> On 21/11/18 09:13 AM, Daniel Golle wrote:
>> Hi!
>>
>> On Thu, Nov 15, 2018 at 07:58:07PM +0100, Daniel Golle wrote:
>>> Hi Jonathan,
>>>
>>> On Thu, Nov 15, 2018 at 11:12:54AM -0500, Jonathan Thibault wrote:
>>>> Greetings,
>>>>
>>>> I would like to propose enabling CONFIG_BRIDGE_VLAN_FILTERING on
>>>> OpenWRT
>>>> releases.
>>> I've suggested that already about a year ago. Now that a significant
>>> amount of targets uses DSA this is even more needed and I think we
>>> should enable it now before the 2019 release is coming up.
>> I'm about to merge this into master with my SoB added as you didn't
>> sign off the patch (which is so trivial that in practise this doesn't
>> matter). Find it in my staging tree [1] and raise your voice in case
>> of any concerns. Otherwise I'll merge it by the end of the week.
> That's perfectly fine.  I'm just happy it's getting merged, OpenWRT just
> got more useful.

I posted a similar patch some time ago and then we agreed to not merge
it, because it increases the kernel size by 4 KBytes compressed:
https://lists.openwrt.org/pipermail/openwrt-devel/2018-April/011715.html

I also would like to have this feature and though about to make this
depend on some sort of feature flag.

As OpenWrt supports very different devices by now, some of them having
just 8MB flash / 32 MB RAM and others having a lot more memory, it would
be good to have some sort for feature flag for the size and activate
such settings based on that.

I have no problem when you merge this patch as is.

Hauke
diff mbox series

Patch

diff --git a/target/linux/generic/config-4.14 
b/target/linux/generic/config-4.14
index 97207cf2eb..7d4750f461 100644
--- a/target/linux/generic/config-4.14
+++ b/target/linux/generic/config-4.14
@@ -627,7 +627,7 @@  CONFIG_BRIDGE=y
  CONFIG_BRIDGE_IGMP_SNOOPING=y
  # CONFIG_BRIDGE_NETFILTER is not set
  # CONFIG_BRIDGE_NF_EBTABLES is not set
-# CONFIG_BRIDGE_VLAN_FILTERING is not set
+CONFIG_BRIDGE_VLAN_FILTERING=y
  # CONFIG_BROADCOM_PHY is not set
  CONFIG_BROKEN_ON_SMP=y
  # CONFIG_BSD_DISKLABEL is not set
diff --git a/target/linux/generic/config-4.9 
b/target/linux/generic/config-4.9
index 979028f04a..860b39b428 100644
--- a/target/linux/generic/config-4.9
+++ b/target/linux/generic/config-4.9
@@ -598,7 +598,7 @@  CONFIG_BRIDGE=y
  CONFIG_BRIDGE_IGMP_SNOOPING=y
  # CONFIG_BRIDGE_NETFILTER is not set
  # CONFIG_BRIDGE_NF_EBTABLES is not set
-# CONFIG_BRIDGE_VLAN_FILTERING is not set
+CONFIG_BRIDGE_VLAN_FILTERING=y
  # CONFIG_BROADCOM_PHY is not set
  CONFIG_BROKEN_ON_SMP=y
  # CONFIG_BSD_DISKLABEL is not set