Re: [PATCH 13/16 net-next v2] net: bridge: guard ARP/RARP proxy and suppression with CONFIG_IPV4

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In net/bridge/br_arp_nd_proxy.c, the patch removes `#if
  IS_ENABLED(CONFIG_INET)` from above the static helper…

--

net: bridge: guard ARP/RARP proxy and suppression with CONFIG_IPV4

This commit replaces the CONFIG_INET guards around the bridge ARP/RARP
proxy and suppression code with CONFIG_IPV4. The reason given is that
CONFIG_IPV4 is becoming optional and separate from the core stack.

> diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
> index da15f4d7c1aef..d739b86974299 100644
> --- a/net/bridge/br_arp_nd_proxy.c
> +++ b/net/bridge/br_arp_nd_proxy.c
> @@ -39,7 +39,6 @@ void br_recalculate_neigh_suppress_enabled(struct net_bridge *br)
>  	br_opt_toggle(br, BROPT_NEIGH_SUPPRESS_ENABLED, neigh_suppress);
>  }
>  
> -#if IS_ENABLED(CONFIG_INET)
>  static bool
>  br_is_neigh_suppress_enabled_vid(const struct net_bridge_port *p, u16 vid)
>  {
> @@ -57,6 +56,7 @@ br_is_neigh_suppress_enabled_vid(const struct net_bridge_port *p, u16 vid)
>  	return br_is_neigh_suppress_enabled(p, v);
>  }
>  
> +#if IS_ENABLED(CONFIG_IPV4)

[Severity: Low]
After this change, the static helper br_is_neigh_suppress_enabled_vid() is
always compiled. Does that give a "defined but not used" warning when
CONFIG_INET=n?

All of its callers are still behind config guards:

  br_do_proxy_suppress_arp()  inside #if IS_ENABLED(CONFIG_IPV4)
  br_do_suppress_nd()         inside #if IS_ENABLED(CONFIG_IPV6)

config IPV4 and config IPV6 are only defined under "if INET" in
net/Kconfig. config BRIDGE has no INET dependency, and br_arp_nd_proxy.o is
always part of bridge-y:

net/bridge/Makefile:
bridge-y	:= br.o br_device.o br_fdb.o br_forward.o br_if.o br_input.o \
			...
			br_netlink_tunnel.o br_arp_nd_proxy.o

With CONFIG_BRIDGE=y/m and CONFIG_INET=n, the file is still built, but
both guarded blocks are compiled out. That leaves the helper with no
callers and triggers -Wunused-function. With CONFIG_WERROR=y, which
defaults to y under COMPILE_TEST, the build fails. At the baseline, the
CONFIG_INET guard covered the helper, so this configuration built without
warnings.

Moving the helper out of the IPv4 block makes sense, because the IPv6
block needs it once IPV4 can be disabled. Could the helper get its own
guard instead, such as #if IS_ENABLED(CONFIG_INET) or
#if IS_ENABLED(CONFIG_IPV4) || IS_ENABLED(CONFIG_IPV6), closed before the
IPv4 block starts?

This still happens at the end of the series. The only configuration that
triggers it is INET=n. INET=y with both IPV4=n and IPV6=n cannot be
selected, because config IPV4 is only user-selectable when IPV6 is enabled
and defaults to INET.

>  static void br_arp_send(struct net_bridge *br, struct net_bridge_port *p,
>  			struct net_device *dev, __be32 dest_ip, __be32 src_ip,
>  			const unsigned char *dest_hw,

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928193046.6698-1-fmancera%40suse.de