Re: [Bridge] [PATCH iproute2-next v3] iplink: bridge: Add support for bridge FDB learning limits

[Petr Machata via Bridge]

(I pruned the CC list, hopefully I didn't leave out anybody who cares.)

Johannes Nixdorf via Bridge  writes:

> Support setting the FDB limit through ip link. The arguments is:
>  - fdb_max_learned_entries: A 32-bit unsigned integer specifying the
> maximum number of learned FDB entries, with 0
> disabling the limit.

...

> Signed-off-by: Johannes Nixdorf 

Code looks good to me. A couple points though:

- The corresponding kernel changes have not yet been merged, have they?
  This patch should obviously only be merged after that happens.

- The UAPI changes should not be part of the patch, the maintainers will
  update themselves.

- The names fdb_n_learned_entries, fdb_max_learned_entries... they are
  somewhat unwieldy IMHO. Just for consideration, I don't feel strongly
  about this:

  Your kconfig option is named BRIDGE_DEFAULT_FDB_MAX_LEARNED, and that
  makes sense to me, because yeah, given the word "learned" in context
  of FDB, "entries" is the obvious continuation, so why mention it
  explicitly. Consider following suit with the other source artifacts --
  the attribute names, struct fields, keywords in this patch.
  "fdb_n_learned", "fdb_max_learned" is IMHO just as understandable and
  will be easier to type.

[Bridge] [PATCH iproute2-next v3] iplink: bridge: Add support for bridge FDB learning limits

[Johannes Nixdorf via Bridge]

Support setting the FDB limit through ip link. The arguments is:
 - fdb_max_learned_entries: A 32-bit unsigned integer specifying the
maximum number of learned FDB entries, with 0
disabling the limit.

Also support reading back the current number of learned FDB entries in
the bridge by this count. The returned value's name is:
 - fdb_n_learned_entries: A 32-bit unsigned integer specifying the
  current number of learned FDB entries.

Example:

 # ip -d -j -p link show br0
[ {
...
"linkinfo": {
"info_kind": "bridge",
"info_data": {
...
"fdb_n_learned_entries": 2,
"fdb_max_learned_entries": 0,
...
}
},
...
} ]
 # ip link set br0 type bridge fdb_max_learned_entries 1024
 # ip -d -j -p link show br0
[ {
...
"linkinfo": {
"info_kind": "bridge",
"info_data": {
...
"fdb_n_learned_entries": 2,
"fdb_max_learned_entries": 1024,
...
}
},
...
} ]

Signed-off-by: Johannes Nixdorf 
---
Changes since v2:
 - Properly split the net-next and iproute2-next threads. (from review)
 - Changed to *_n_* instead of *_cur_*. (from review)
 - Use strcmp() instead of matches(). (from review)
 - Made names in code and documentation consistent. (from review)
 - Various documentation fixes. (from review)

Changes since v1:
 - Sent out the first corresponding iproute2 patches.

net-next v3: 
https://lore.kernel.org/netdev/20230905-fdb_limit-v3-0-7597cd500...@avm.de/

v2: https://lore.kernel.org/netdev/20230619071444.14625-1-jnixdorf-...@avm.de/
v1: https://lore.kernel.org/netdev/20230515085046.4457-1-jnixdorf-...@avm.de/
---
 include/uapi/linux/if_link.h |  2 ++
 ip/iplink_bridge.c   | 21 +
 man/man8/ip-link.8.in| 10 ++
 3 files changed, 33 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c2ca7a6add0e..51cf58e3171c 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -508,6 +508,8 @@ enum {
IFLA_BR_VLAN_STATS_PER_PORT,
IFLA_BR_MULTI_BOOLOPT,
IFLA_BR_MCAST_QUERIER_STATE,
+   IFLA_BR_FDB_N_LEARNED_ENTRIES,
+   IFLA_BR_FDB_MAX_LEARNED_ENTRIES,
__IFLA_BR_MAX,
 };
 
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 7e4e62c81c0c..f08754618e0f 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -34,6 +34,7 @@ static void print_explain(FILE *f)
" [ group_fwd_mask MASK ]\n"
" [ group_address ADDRESS ]\n"
" [ no_linklocal_learn NO_LINKLOCAL_LEARN ]\n"
+   " [ fdb_max_learned_entries 
FDB_MAX_LEARNED_ENTRIES ]\n"
" [ vlan_filtering VLAN_FILTERING ]\n"
" [ vlan_protocol VLAN_PROTOCOL ]\n"
" [ vlan_default_pvid VLAN_DEFAULT_PVID ]\n"
@@ -168,6 +169,14 @@ static int bridge_parse_opt(struct link_util *lu, int 
argc, char **argv,
bm.optval |= no_ll_learn_bit;
else
bm.optval &= ~no_ll_learn_bit;
+   } else if (strcmp(*argv, "fdb_max_learned_entries") == 0) {
+   __u32 fdb_max_learned_entries;
+
+   NEXT_ARG();
+   if (get_u32(_max_learned_entries, *argv, 0))
+   invarg("invalid fdb_max_learned_entries", 
*argv);
+
+   addattr32(n, 1024, IFLA_BR_FDB_MAX_LEARNED_ENTRIES, 
fdb_max_learned_entries);
} else if (matches(*argv, "fdb_flush") == 0) {
addattr(n, 1024, IFLA_BR_FDB_FLUSH);
} else if (matches(*argv, "vlan_default_pvid") == 0) {
@@ -544,6 +553,18 @@ static void bridge_print_opt(struct link_util *lu, FILE 
*f, struct rtattr *tb[])
if (tb[IFLA_BR_GC_TIMER])
_bridge_print_timer(f, "gc_timer", tb[IFLA_BR_GC_TIMER]);
 
+   if (tb[IFLA_BR_FDB_N_LEARNED_ENTRIES])
+   print_uint(PRINT_ANY,
+  "fdb_n_learned_entries",
+  "fdb_n_learned_entries %u ",
+  rta_getattr_u32(tb[IFLA_BR_FDB_N_LEARNED_ENTRIES]));
+
+   if (tb[IFLA_BR_FDB_MAX_LEARNED_ENTRIES])
+   print_uint(PRINT_ANY,
+  "fdb_max_learned_entries",
+  "fdb_max_learned_entries %u ",
+  
rta_getattr_u32(tb[IFLA_BR_FDB_MAX_LEARNED_ENTRIES]));
+
if (tb[IFLA_BR_VLAN_DEFAULT_PVID])
print_uint(PRINT_ANY,
   "vlan_default_pvid",
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 7365d0c6b14f..184ae9183712 100644
--- a/man/man8/ip-link.8.in
+++