Created attachment 249661 [details] patches and example ngctl for testing The removal of NG_BRIDGE_MAX_LINKS in review D21803 has made it possible to have way more than 32 connections to an ng_bridge. But there isn't any (easy) way for userland to track available link names and requires listing all existing [up]links or to just guess a number until one succeeds. There appears to never be a need to know the link number of the hook on the ng_bridge. When an ng_bridge shuts down it disconnects all hooks. If you know the path of your ng_eiface, ng_socket, ng_ether, etc. you have a way to `rmhook` without the link path. That is you can remove via either: rmhook <ng_bridge path> [up]linkX or with rmhook <ng_eiface path> ether Everything else that connects to ng_bridge has a similar option. This change, however, is about removing the need to specify an [up]link number when creating a connection to the ng_bridge. Using ng_eiface again it means you can use: connect <ng_eiface path> <ng_bridge path> ether link The ng_bridge will auto assign the lowest available [up]link number that is valid. The number space for [up]link is as large as an int, limited by the fact that all netgraph nodes use an int to track the number of connected hooks they have, nd_numhooks in struct ng_node. This turns out to be a convenient fact when introducing unr(9) as I can just initialize to INT_MAX. But it also means you could just take advantage of that and create a numbering scheme in user space. For example: 0,1 always reserved for ng_ether (which requires 2 hooks) 16 - N reserved for bhyve N - INT_MAX reserved for jails The only issue is that 3rd party scripts would have to agree (e.g. ezjail, vm-bhyve, etc.) Each script would have to scan and when it finds an available [up]link number hope its still there when it attempts the `connect`. "uplink" was introduced with review D23963. I have preserved the requirement you still can not have "uplink0". That will be automatically handled if you connect and request "uplink", it will give you "uplink1" or higher depending on availability but never "uplink0". Because [up]link number is not required it also becomes possible to inspect a netgraph node type and based on that determine the `peerhook` argument to connect to an ng_bridge. I added a change to bhyve to do exactly that. This allows for a simplification when using netgraph with bhyve, which became possible with review D24620. Instead of: -t netgraph,socket=vm0,path=vmbridge:,hook=vm0link,peerhook=link0 You can shorten to just: -t netgraph,path=vmbridge: Instead of: -t netgraph,socket=vm1,path=igb0:,hook=vm1link,peerhook=lower You can shorten to just: -t netgraph,path=igb0: I'm just removing the need for `peerhook`. No need to specify `socket` or `hook` as the code exists today. Without this you could start just passing "link" for peerhook when using an ng_bridge. It makes the bhyve change optional, and I'm ok if its removed, I mostly care about not having to deal with numbering links for my jails that use ng_eiface. This change is not 100% backward compatible though. The unr(9) is shared by `link` and `uplink`. The current code will allow `link1` and `uplink1` and this change will not. The total number of [up]links is unchanged. But bear in mind uplink is for a physical interface connected to an ng_bridge, and the expectation is that you have only one of these, so it would be wasteful to have a separate unr(9) for just "uplink". But other than that it does still allow users to specify [up]link number. The tarball contains patches for CURRENT and stable/14. stable/13 still uses the the array of 32 hooks with NG_BRIDGE_MAX_LINKS. It also contains some simple tests to try with ngctl(8).
Hi David! It would be nice to post a patch for ng_bridge(4) on the https://reviews.freebsd.org/. You can do this via the web interface according to the instructions: https://wiki.freebsd.org/Phabricator (Create a Revision via Web Interface) A quick inspection of the patch showed that I have something to recommend.
Added review D44615 as requested.
Thanks for submission! Already working on comments in the review.
attached patches out of date. review has current.
Created attachment 249705 [details] updated patch for CURRENT updating patch for CURRENT to match the review
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=86a6393a7d6766875a9e03daa0273a2e55faacdd commit 86a6393a7d6766875a9e03daa0273a2e55faacdd Author: David Marker <dave@freedave.net> AuthorDate: 2024-04-08 17:48:22 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> CommitDate: 2024-04-08 17:48:22 +0000 ng_bridge: allow to automatically assign numbers to new hooks This will allow a userland machinery that orchestrates a bridge (e.g. a jail or vm manager) to not double the number allocation logic. See bug 278130 for longer description and examples. Reviewed by: glebius, afedorov Differential Revision: https://reviews.freebsd.org/D44615 PR: 278130 share/man/man4/ng_bridge.4 | 13 +++++- sys/netgraph/ng_bridge.c | 104 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 93 insertions(+), 24 deletions(-)