[v5,0/3] batman-adv: add dynamic, bridged-in TT VID detection

Message ID 20250201043725.15217-1-linus.luessing@c0d3.blue (mailing list archive)
Headers
Series batman-adv: add dynamic, bridged-in TT VID detection |

Message

Linus Lüssing Feb. 1, 2025, 4:31 a.m. UTC
  For one thing the following patchest is supposed to mitigate the
regression of extra OGM overhead that we accidentally introduced with
the introduction of the compatibility version 15, the TVLV support it
provided, together with TT VLAN support. The addition of these TVLVs
roughly doubled the OGM overhead back then, also/mainly because the
8021q and Linux bridge modules would add TT VLAN IDs 0 and 1, even if
no one is actually using them. While the 8021q VLAN 0 ID addition was
addressed in upstream already, PATCH 3/3 tries to avoid unnecessary
VLAN additions from the bridge and only adds such VLANs if they were
detected on actual traffic.

In preparation of this the internal VLAN creation was overhauled in
PATCH 1/3. This also in theory allows detecting new VLAN IDs dynamically
from traffic from bridged-in traffic. So far, VLAN IDs could only be
used which were anticipated and configured on top of bat0 by the
administrator.

However the detection of VLANs from bridged-in clients is by default
disabled for now through PATCH 2/3. This patch adds a configurable limit
for such snooped VLAN IDs, defaulting to zero for now. The issue is that
each added VLAN still increases the OGM size considerably in the current
protocol, so it is not advised to add many VLANs at the moment, without
a bigger upgrade of the protocol. Also there is still an outstanding
issue with temporarily broken broadcast traffic upon adding a new VLAN if
BLA is enabled at the same time. Therefore defaulting to zero snooped
VLANs from bridged-in clients for now.

Regards, Linus

---

v5:
* rebased to current main branch:
  -> PATCH 3/3 needed to readd the include for soft-interface.h for
     batadv_softif_get_bridge() since commit
     "50eddf397ac3 batman-adv: netlink: reduce duplicate code by returning interfaces"

v4:
* reworking PATCH 3/3:
  * removing the added exception for VID 0 again, addressed upstream now
  * replacing the specific VID 1 exception for the bridge default PVID
    with a more generic solution: a user might change the PVID or add
    other, untagged VLANs on an access port which we should also ignore;
    instead always avoid such kernel event additions if a bridge is on
    top of bat0

v3:
* fixing refcounting, removing an unnecessary kref_get() in PATCH 1/3
* adding PATCH 2/3 + PATCH 3/3
* resubmitting without the RFC tag

v2: fix a typo, a missing "to" in the commit message
  

Comments

Sven Eckelmann Feb. 1, 2025, 9:42 a.m. UTC | #1
On Saturday, 1 February 2025 05:31:27 GMT+1 Linus Lüssing wrote:
> v5:
> * rebased to current main branch:
>   -> PATCH 3/3 needed to readd the include for soft-interface.h for
>      batadv_softif_get_bridge() since commit
>      "50eddf397ac3 batman-adv: netlink: reduce duplicate code by returning interfaces"

I see also following reports:

ecsv/pu: headers
----------------

    diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
    index 051e7fce..30f65708 100644
    --- a/net/batman-adv/soft-interface.c
    +++ b/net/batman-adv/soft-interface.c
    @@ -23,6 +23,7 @@
     #include <linux/kref.h>
     #include <linux/list.h>
     #include <linux/lockdep.h>
    +#include <linux/net.h>
     #include <linux/netdevice.h>
     #include <linux/netlink.h>
     #include <linux/percpu.h>

ecsv/pu: kerneldoc ./net/batman-adv/soft-interface.c
----------------------------------------------------

    ./net/batman-adv/soft-interface.c:566: warning: Function parameter or struct member 'own' not described in 'batadv_softif_create_vlan'
    ./net/batman-adv/soft-interface.c:626: warning: Function parameter or struct member 'own' not described in 'batadv_softif_vlan_get_or_create'

ecsv/pu: unused_symbols
-----------------------

cfg:

 * unused_symbols cfg: BLA=n DAT=n DEBUG=n TRACING=n NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=n DEBUG=n TRACING=n NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=n DEBUG=n TRACING=y NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=n DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=n DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=n DEBUG=y TRACING=n NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=n DEBUG=y TRACING=y NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=n DEBUG=y TRACING=y NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=y DEBUG=n TRACING=n NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=y DEBUG=n TRACING=n NC=y MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=y DEBUG=n TRACING=y NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=y DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=y DEBUG=y TRACING=n NC=y MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=y DEBUG=y TRACING=y NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=n DAT=y DEBUG=y TRACING=y NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=n DAT=y DEBUG=y TRACING=y NC=y MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=y DAT=n DEBUG=n TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=n DEBUG=n TRACING=n NC=y MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=y DAT=n DEBUG=n TRACING=y NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=n DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=n DEBUG=y TRACING=n NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=n DEBUG=y TRACING=n NC=y MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=y DAT=n DEBUG=y TRACING=y NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=y DEBUG=n TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=y DEBUG=n TRACING=n NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=y DEBUG=n TRACING=y NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=y DAT=y DEBUG=n TRACING=y NC=y MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=y DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=n
 * unused_symbols cfg: BLA=y DAT=y DEBUG=y TRACING=n NC=n MCAST=n BATMAN_V=y
 * unused_symbols cfg: BLA=y DAT=y DEBUG=y TRACING=y NC=n MCAST=n BATMAN_V=y

    batadv_softif_get_bridge


I am guessing I must just ignore the latter because it is only relevant for 
non-mcast build and the noise from the extra #ifdef around "static" isn't 
worth it.

The the linux/net.h "dependency" came with the second patch

Btw. I am still waiting for reviews from Antonio

Kind regards,
	Sven