Re: [PATCH net-next 2/2] bridge: Add a sysctl to limit new brides FDB entries

On 15/05/2023 14:27, Johannes Nixdorf wrote:

On Mon, May 15, 2023 at 12:35:47PM +0300, Nikolay Aleksandrov wrote:

quoted

On 15/05/2023 11:50, Johannes Nixdorf wrote:

quoted

This is a convenience setting, which allows the administrator to limit
the default limit of FDB entries for all created bridges, instead of
having to set it for each created bridge using the netlink property.

The setting is network namespace local, and defaults to 0, which means
unlimited, for backwards compatibility reasons.

Signed-off-by: Johannes Nixdorf <redacted>
---
 net/bridge/br.c         | 83 +++++++++++++++++++++++++++++++++++++++++
 net/bridge/br_device.c  |  4 +-
 net/bridge/br_private.h |  9 +++++
 3 files changed, 95 insertions(+), 1 deletion(-)

The bridge doesn't need private sysctls. Netlink is enough.
Nacked-by: Nikolay Aleksandrov [off-list ref]

Fair enough.

I originally included the setting so there is a global setting an
administrator could toggle instead of having to hunt down each process
that might create a bridge, and teaching them to create them with an
FDB limit.

Does any of the following alternatives sound acceptable to you?:
 - Having the default limit (instead of the proposed default to unlimited)
   configurable in Kbuild. This would solve our problem, as we build
   our kernels ourselves, but I don't know whether putting a limit there
   would be acceptable for e.g. distributions.

I don't mind, but it would be useless for everyone else. Kernels would be built
without that limit set.

 - Hardcoding a default limit != 0. I was afraid I'd break someones
   use-case with far too large bridged networks if I don't default to
   unlimited, but if you maintainers have a number in mind with which
   you don't see a problem, I'd be fine with it as well.

(Sorry for sending this mail twice, I accidentally dropped the list and
CC on the fist try)

Right, that has been discussed before. So far there hasn't been any good
option, so I'd say for the time being (or unless you have some better idea)
we should stick with the netlink max attribute and distributions/admins
would have to set it on bridge creation. We could add a warning when creating
a bridge without fdb limit to remind people that it's advisable to set it.
That warning can be removed when we come up with a proper solution.