[v3,3/3] batman-adv: avoid adding VLAN IDs 0 + 1 through kernel events

Message ID 20241202053511.326-4-linus.luessing@c0d3.blue (mailing list archive)
State New
Delegated to: Antonio Quartulli
Headers
Series add dynamic, bridged-in TT VID detection support |

Commit Message

Linus Lüssing Dec. 2, 2024, 5:05 a.m. UTC
  Currently the 8021q module always registers VLAN ID 0 and the Linux
bridge always registers VLAN ID 1 if bat0 is added to a bridge
(probably as a quirk for hardware network/switch device drivers).
Even though we might not actually use them over the mesh.
The issue is that any extra VLAN currently increases our own
OGM protocol overhead quite a bit, so we want to avoid that
by only adding VLANs that we are sure someone will be using.
So only add VLAN IDs 0 and 1 through snooping of actual, VLAN tagged
traffic, not through kernel internal network events.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/batman-adv/soft-interface.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Comments

Sven Eckelmann Dec. 2, 2024, 12:48 p.m. UTC | #1
On Monday, 2 December 2024 06:05:22 CET Linus Lüssing wrote:
> Currently the 8021q module always registers VLAN ID 0

To ensure that HW filters would forward priority frames. If you follow the 
argumentation in 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ad1afb00393915a51c21b1ae8704562bf036855f 
then you can assume that the no-VLAN TT table entries should be used for 
VLAN 0 because it is just for priority tagging - and not an actual VLAN.

> and the Linux
> bridge always registers VLAN ID 1 if bat0 is added to a bridge
> (probably as a quirk for hardware network/switch device drivers).

No, it is not a quirk. It just adds the default PVID of a bridge.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/bridge/br_vlan.c?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95#n1248

This sets as default PVID the 1:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/bridge/br_vlan.c?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95#n1218

And the default PVID could be changed by:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/bridge/br_vlan.c?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95#n1093

It will "disable" the PVID in case it is 0

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/bridge/br_vlan.c?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95#n1104

Kind regards,
	Sven
  

Patch

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index d08f5e99f39f..7a6287575505 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -699,6 +699,20 @@  static int batadv_interface_add_vid(struct net_device *dev, __be16 proto,
 	if (proto != htons(ETH_P_8021Q))
 		return -EINVAL;
 
+	/*
+	 * Currently the 8021q module always registers VLAN ID 0 and the Linux
+	 * bridge always registers VLAN ID 1 if bat0 is added to a bridge
+	 * (probably as a quirk for hardware network/switch device drivers).
+	 * Even though we might not actually use them over the mesh.
+	 * The issue is that any extra VLAN currently increases our own
+	 * OGM protocol overhead quite a bit, so we want to avoid that
+	 * by only adding VLANs that we are sure someone will be using.
+	 * So only add VLAN IDs 0 and 1 through snooping of actual, VLAN tagged
+	 * traffic, not through kernel internal network events.
+	 */
+	if (vid == 0 || vid == 1)
+		return 0;
+
 	vid |= BATADV_VLAN_HAS_TAG;
 
 	return batadv_softif_create_vlan_own(bat_priv, vid);
@@ -727,6 +741,9 @@  static int batadv_interface_kill_vid(struct net_device *dev, __be16 proto,
 	if (proto != htons(ETH_P_8021Q))
 		return -EINVAL;
 
+	if (vid == 0 || vid == 1)
+		return 0;
+
 	batadv_softif_destroy_vlan_own(bat_priv, vid | BATADV_VLAN_HAS_TAG);
 	return 0;
 }