Re: [PATCH 01/16 net-next v2] ipv4: introduce CONFIG_IPV4 to decouple the IPv4 stack

[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] The patch moves route.o, arp.o, icmp and the other IPv4-only
  objects under obj-$(CONFIG_IPV4) and adds `depends on IPV4` / `depends…

--

ipv4: introduce CONFIG_IPV4 to decouple the IPv4 stack

This commit adds a CONFIG_IPV4 symbol, which is def_bool y for now. It
moves the IPv4-only objects in net/ipv4/Makefile under obj-$(CONFIG_IPV4).
It also adds IPV4 (or IPV4 || !INET) dependencies to a number of drivers,
filesystems and networking features that use those symbols.

> diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
> index 06e21c26b76f..8b148be79fa4 100644
> --- a/net/ipv4/Makefile
> +++ b/net/ipv4/Makefile
> @@ -3,18 +3,17 @@
>  # Makefile for the Linux TCP/IP (INET) layer.
>  #
>
> -obj-y     := route.o inetpeer.o protocol.o \
> -	     ip_input.o ip_fragment.o ip_forward.o ip_options.o \
> -	     ip_output.o ip_sockglue.o inet_hashtables.o \
> -	     inet_timewait_sock.o inet_connection_sock.o \
> -	     tcp.o tcp_input.o tcp_output.o tcp_timer.o tcp_ipv4.o \
> +obj-y     := inetpeer.o protocol.o inet_hashtables.o inet_timewait_sock.o \
> +	     inet_connection_sock.o tcp.o tcp_input.o tcp_output.o tcp_timer.o \
>  	     tcp_minisocks.o tcp_cong.o tcp_metrics.o tcp_fastopen.o \
> -	     tcp_recovery.o tcp_ulp.o \
> -	     tcp_offload.o tcp_plb.o datagram.o raw.o udp.o \
> -	     udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
> -	     fib_frontend.o fib_semantics.o fib_trie.o fib_notifier.o \
> -	     inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o \
> -	     metrics.o netlink.o nexthop.o udp_tunnel_stub.o
> +	     tcp_recovery.o tcp_ulp.o tcp_offload.o tcp_plb.o raw.o udp.o \
> +	     udp_offload.o af_inet.o inet_fragment.o ping.o ip_tunnel_core.o \
> +	     gre_offload.o metrics.o netlink.o nexthop.o udp_tunnel_stub.o
> +
> +obj-$(CONFIG_IPV4) += route.o ip_input.o ip_fragment.o ip_forward.o ip_options.o \
> +		      ip_sockglue.o tcp_ipv4.o datagram.o icmp.o arp.o devinet.o \
> +		      igmp.o fib_notifier.o ip_output.o fib_frontend.o \
> +		      fib_semantics.o fib_trie.o

[Severity: Low]
Is SUNVNET_COMMON missing from the list of consumers that gained an IPV4
dependency? Its Kconfig entry is unchanged:

drivers/net/ethernet/sun/Kconfig:
config SUNVNET_COMMON
	tristate "Common routines to support Sun Virtual Networking"
	depends on SUN_LDOMS
	depends on INET
	default m

vnet_start_xmit() in drivers/net/ethernet/sun/sunvnet_common.c calls
icmp_ndo_send(). The only guard is a runtime protocol check:

	if (skb->protocol == htons(ETH_P_IP))
		icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
			      htonl(localmtu));

include/net/icmp.h has no !CONFIG_IPV4 stub for icmp_ndo_send() or
__icmp_send(). The "Core Netfilter Configuration" menu now depends on
IPV4, so NF_NAT is n when IPV4=n. In that case icmp_ndo_send() is the
inline wrapper that calls __icmp_send() directly.

With IPV4 set to def_bool y, this can't happen yet. After the later
"ipv4: make CONFIG_IPV4 boolean" change, __icmp_send() is defined only in
net/ipv4/icmp_ipv4.c, and that file is built only through
obj-$(CONFIG_IPV4).

Consider a sparc64 config with SUN_LDOMS=y, INET=y, IPV6=y and IPV4=n.
SUNVNET_COMMON defaults to m there. Would that config fail at modpost
with an undefined __icmp_send?

Nothing later in the series seems to add an IPV4 dependency to
SUNVNET_COMMON, or a guard or stub for icmp_ndo_send(). Would a
"depends on IPV4 || !INET" line, like the one used for sfc, stmmac and
mlxsw, be appropriate here?

Other drivers with a similar pattern, such as bnxt, nfp and cnic, look
fine at the end of the series. They reach ip_route_output_flow(), and
"net: core: add IPv4 fallback stubs and guards for CONFIG_IPV4=n" stubs
that function in include/net/route.h to return ERR_PTR(-EAFNOSUPPORT).

>
>  obj-$(CONFIG_NET_IP_TUNNEL) += ip_tunnel.o
>  obj-$(CONFIG_SYSCTL) += sysctl_net_ipv4.o

[ ... ]

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