Date: Mon, 5 May 2025 22:16:47 GMT From: Lexi Winter <ivy@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 0a1294f6c610 - main - bridge: allow IP addresses on members to be disabled Message-ID: <202505052216.545MGljO049256@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by ivy: URL: https://cgit.FreeBSD.org/src/commit/?id=0a1294f6c610948d7447ae276df74a6d5269b62e commit 0a1294f6c610948d7447ae276df74a6d5269b62e Author: Lexi Winter <ivy@FreeBSD.org> AuthorDate: 2025-05-05 21:44:44 +0000 Commit: Lexi Winter <ivy@FreeBSD.org> CommitDate: 2025-05-05 21:47:36 +0000 bridge: allow IP addresses on members to be disabled add a new sysctl, net.link.bridge.member_ifaddrs, which defaults to 1. if it is set to 1, bridge behaviour is unchanged. if it is set to 0: - an interface which has AF_INET6 or AF_INET addresses assigned cannot be added to a bridge. - an interface in a bridge cannot have an AF_INET6 or AF_INET address assigned to it. - the bridge will no longer consider the lladdrs on bridge members to be local addresses, i.e. frames sent to member lladdrs will not be processed by the host. update bridge.4 to document this behaviour, as well as the existing recommendation that IP addresses should not be configured on bridge members anyway, even if it currently partially works. in testing, setting this to 0 on a bridge with 50 member interfaces improved throughput by 22% (4.61Gb/s -> 5.67Gb/s) across two member epairs due to eliding the bridge member list walk in GRAB_OUR_PACKETS. Reviewed by: kp, des Approved by: des (mentor) Differential Revision: https://reviews.freebsd.org/D49995 --- share/man/man4/bridge.4 | 15 +++++++- sys/net/if_bridge.c | 61 +++++++++++++++++++++++++++----- sys/net/if_bridgevar.h | 1 + sys/net/if_ethersubr.c | 1 + sys/netinet/in.c | 8 +++++ sys/netinet6/in6.c | 8 +++++ tests/sys/net/if_bridge_test.sh | 78 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 162 insertions(+), 10 deletions(-) diff --git a/share/man/man4/bridge.4 b/share/man/man4/bridge.4 index b741ec0b73c8..2c3bfd6aedfa 100644 --- a/share/man/man4/bridge.4 +++ b/share/man/man4/bridge.4 @@ -36,7 +36,7 @@ .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE .\" POSSIBILITY OF SUCH DAMAGE. .\" -.Dd April 10, 2024 +.Dd May 5, 2025 .Dt IF_BRIDGE 4 .Os .Sh NAME @@ -158,6 +158,19 @@ This can be used to multiplex the input of two or more interfaces into a single stream. This is useful for reconstructing the traffic for network taps that transmit the RX/TX signals out through two separate interfaces. +.Pp +To allow the host to communicate with bridge members, IP addresses +should be assigned to the +.Nm +interface itself, not to the bridge's member interfaces. +Assigning IP addresses to bridge member interfaces is unsupported, but +for backward compatibility, it is permitted if the +.Xr sysctl 8 +variable +.Va net.link.bridge.member_ifaddrs +is set to 1, which is the default. +In a future release, this sysctl may be set to 0 by default, or may be +removed entirely. .Sh IPV6 SUPPORT .Nm supports the diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index c587f0d0f70a..199418c4aa99 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -336,6 +336,7 @@ static void bridge_rtdelete(struct bridge_softc *, struct ifnet *ifp, int); static void bridge_forward(struct bridge_softc *, struct bridge_iflist *, struct mbuf *m); +static bool bridge_member_ifaddrs(void); static void bridge_timer(void *); @@ -502,6 +503,19 @@ SYSCTL_BOOL(_net_link_bridge, OID_AUTO, log_mac_flap, CTLFLAG_RW | CTLFLAG_VNET, &VNET_NAME(log_mac_flap), true, "Log MAC address port flapping"); +/* allow IP addresses on bridge members */ +VNET_DEFINE_STATIC(bool, member_ifaddrs) = true; +#define V_member_ifaddrs VNET(member_ifaddrs) +SYSCTL_BOOL(_net_link_bridge, OID_AUTO, member_ifaddrs, + CTLFLAG_RW | CTLFLAG_VNET, &VNET_NAME(member_ifaddrs), true, + "Allow layer 3 addresses on bridge members"); + +static bool +bridge_member_ifaddrs(void) +{ + return (V_member_ifaddrs); +} + VNET_DEFINE_STATIC(int, log_interval) = 5; VNET_DEFINE_STATIC(int, log_count) = 0; VNET_DEFINE_STATIC(struct timeval, log_last) = { 0 }; @@ -665,6 +679,7 @@ bridge_modevent(module_t mod, int type, void *data) bridge_dn_p = bridge_dummynet; bridge_same_p = bridge_same; bridge_get_softc_p = bridge_get_softc; + bridge_member_ifaddrs_p = bridge_member_ifaddrs; bridge_detach_cookie = EVENTHANDLER_REGISTER( ifnet_departure_event, bridge_ifdetach, NULL, EVENTHANDLER_PRI_ANY); @@ -675,6 +690,7 @@ bridge_modevent(module_t mod, int type, void *data) bridge_dn_p = NULL; bridge_same_p = NULL; bridge_get_softc_p = NULL; + bridge_member_ifaddrs_p = NULL; break; default: return (EOPNOTSUPP); @@ -1313,6 +1329,25 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg) return (EINVAL); } + /* + * If member_ifaddrs is disabled, do not allow an interface with + * assigned IP addresses to be added to a bridge. + */ + if (!V_member_ifaddrs) { + struct ifaddr *ifa; + + CK_STAILQ_FOREACH(ifa, &ifs->if_addrhead, ifa_link) { +#ifdef INET + if (ifa->ifa_addr->sa_family == AF_INET) + return (EINVAL); +#endif +#ifdef INET6 + if (ifa->ifa_addr->sa_family == AF_INET6) + return (EINVAL); +#endif + } + } + #ifdef INET6 /* * Two valid inet6 addresses with link-local scope must not be @@ -2742,17 +2777,25 @@ bridge_input(struct ifnet *ifp, struct mbuf *m) do { GRAB_OUR_PACKETS(bifp) } while (0); /* - * Give a chance for ifp at first priority. This will help when the - * packet comes through the interface like VLAN's with the same MACs - * on several interfaces from the same bridge. This also will save - * some CPU cycles in case the destination interface and the input - * interface (eq ifp) are the same. + * We only need to check members interfaces if member_ifaddrs is + * enabled; otherwise we should have never traffic destined for a + * member's lladdr. */ - do { GRAB_OUR_PACKETS(ifp) } while (0); - /* Now check the all bridge members. */ - CK_LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) { - GRAB_OUR_PACKETS(bif2->bif_ifp) + if (V_member_ifaddrs) { + /* + * Give a chance for ifp at first priority. This will help when + * the packet comes through the interface like VLAN's with the + * same MACs on several interfaces from the same bridge. This + * also will save some CPU cycles in case the destination + * interface and the input interface (eq ifp) are the same. + */ + do { GRAB_OUR_PACKETS(ifp) } while (0); + + /* Now check the all bridge members. */ + CK_LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) { + GRAB_OUR_PACKETS(bif2->bif_ifp) + } } #undef CARP_CHECK_WE_ARE_DST diff --git a/sys/net/if_bridgevar.h b/sys/net/if_bridgevar.h index a77ac21c5d1d..90beb6c96d82 100644 --- a/sys/net/if_bridgevar.h +++ b/sys/net/if_bridgevar.h @@ -325,6 +325,7 @@ struct ifbpstpconf { extern void (*bridge_dn_p)(struct mbuf *, struct ifnet *); extern bool (*bridge_same_p)(const void *, const void *); extern void *(*bridge_get_softc_p)(struct ifnet *); +extern bool (*bridge_member_ifaddrs_p)(void); #endif /* _KERNEL */ diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c index 2397b5ff2090..cb858f20f3b6 100644 --- a/sys/net/if_ethersubr.c +++ b/sys/net/if_ethersubr.c @@ -112,6 +112,7 @@ void (*vlan_input_p)(struct ifnet *, struct mbuf *); void (*bridge_dn_p)(struct mbuf *, struct ifnet *); bool (*bridge_same_p)(const void *, const void *); void *(*bridge_get_softc_p)(struct ifnet *); +bool (*bridge_member_ifaddrs_p)(void); /* if_lagg(4) support */ struct mbuf *(*lagg_input_ethernet_p)(struct ifnet *, struct mbuf *); diff --git a/sys/netinet/in.c b/sys/netinet/in.c index 2fcbff8611ff..963449d4b4b1 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -57,6 +57,7 @@ #include <net/if_llatbl.h> #include <net/if_private.h> #include <net/if_types.h> +#include <net/if_bridgevar.h> #include <net/route.h> #include <net/route/nhop.h> #include <net/route/route_ctl.h> @@ -518,6 +519,13 @@ in_aifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct ucred *cred return (error); #endif + /* + * Check if bridge wants to allow adding addrs to member interfaces. + */ + if (ifp->if_bridge && bridge_member_ifaddrs_p && + !bridge_member_ifaddrs_p()) + return (EINVAL); + /* * See whether address already exist. */ diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c index 00a0b40154d2..62d00196e56b 100644 --- a/sys/netinet6/in6.c +++ b/sys/netinet6/in6.c @@ -86,6 +86,7 @@ #include <net/if_var.h> #include <net/if_private.h> #include <net/if_types.h> +#include <net/if_bridgevar.h> #include <net/route.h> #include <net/route/route_ctl.h> #include <net/route/nhop.h> @@ -1234,6 +1235,13 @@ in6_addifaddr(struct ifnet *ifp, struct in6_aliasreq *ifra, struct in6_ifaddr *i int carp_attached = 0; int error; + /* Check if this interface is a bridge member */ + if (ifp->if_bridge && bridge_member_ifaddrs_p && + !bridge_member_ifaddrs_p()) { + error = EINVAL; + goto out; + } + /* * first, make or update the interface address structure, * and link it to the list. diff --git a/tests/sys/net/if_bridge_test.sh b/tests/sys/net/if_bridge_test.sh index 46ebb17edbdc..f9a36126fe59 100755 --- a/tests/sys/net/if_bridge_test.sh +++ b/tests/sys/net/if_bridge_test.sh @@ -703,6 +703,82 @@ many_bridge_members_cleanup() vnet_cleanup } +atf_test_case "member_ifaddrs_enabled" "cleanup" +member_ifaddrs_enabled_head() +{ + atf_set descr 'bridge with member_ifaddrs=1' + atf_set require.user root +} + +member_ifaddrs_enabled_body() +{ + vnet_init + vnet_init_bridge + + ep=$(vnet_mkepair) + ifconfig ${ep}a inet 192.0.2.1/24 up + + vnet_mkjail one ${ep}b + jexec one sysctl net.link.bridge.member_ifaddrs=1 + jexec one ifconfig ${ep}b inet 192.0.2.2/24 up + jexec one ifconfig bridge0 create addm ${ep}b + + atf_check -s exit:0 -o ignore ping -c3 -t1 192.0.2.2 +} + +member_ifaddrs_enabled_cleanup() +{ + vnet_cleanup +} + +atf_test_case "member_ifaddrs_disabled" "cleanup" +member_ifaddrs_disabled_head() +{ + atf_set descr 'bridge with member_ifaddrs=0' + atf_set require.user root +} + +member_ifaddrs_disabled_body() +{ + vnet_init + vnet_init_bridge + + vnet_mkjail one + jexec one sysctl net.link.bridge.member_ifaddrs=0 + + bridge=$(jexec one ifconfig bridge create) + + # adding an interface with an IPv4 address + ep=$(jexec one ifconfig epair create) + jexec one ifconfig ${ep} 192.0.2.1/32 + atf_check -s exit:1 -e ignore jexec one ifconfig ${bridge} addm ${ep} + + # adding an interface with an IPv6 address + ep=$(jexec one ifconfig epair create) + jexec one ifconfig ${ep} inet6 2001:db8::1/128 + atf_check -s exit:1 -e ignore jexec one ifconfig ${bridge} addm ${ep} + + # adding an interface with an IPv6 link-local address + ep=$(jexec one ifconfig epair create) + jexec one ifconfig ${ep} inet6 -ifdisabled auto_linklocal up + atf_check -s exit:1 -e ignore jexec one ifconfig ${bridge} addm ${ep} + + # adding an IPv4 address to a member + ep=$(jexec one ifconfig epair create) + jexec one ifconfig ${bridge} addm ${ep} + atf_check -s exit:1 -e ignore jexec one ifconfig ${ep} inet 192.0.2.2/32 + + # adding an IPv6 address to a member + ep=$(jexec one ifconfig epair create) + jexec one ifconfig ${bridge} addm ${ep} + atf_check -s exit:1 -e ignore jexec one ifconfig ${ep} inet6 2001:db8::1/128 +} + +member_ifaddrs_disabled_cleanup() +{ + vnet_cleanup +} + atf_init_test_cases() { atf_add_test_case "bridge_transmit_ipv4_unicast" @@ -718,4 +794,6 @@ atf_init_test_cases() atf_add_test_case "mtu" atf_add_test_case "vlan" atf_add_test_case "many_bridge_members" + atf_add_test_case "member_ifaddrs_enabled" + atf_add_test_case "member_ifaddrs_disabled" }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505052216.545MGljO049256>