Re: [PATCH 09/16 net-next v3] ipv4: fib: split common nexthop logic to fib_core.c

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] net/core/Makefile adds fib_core.o to the unconditional obj-y list
  next to fib_notifier.o, so it is built into every CONFIG_NET kernel,…
- [Low] The patch rewrote the section comment in include/net/ip_fib.h
  from "/* Exported by fib_frontend.c */" to "/* Exported by…

--

ipv4: fib: split common nexthop logic to fib_core.c

This moves the shared nexthop init/release helpers, fib_nexthop_info(),
fib_add_nexthop(), rtm_ipv4_policy and ip_valid_fib_dump_req() out of
net/ipv4 into a new net/core/fib_core.c. The goal is to let IPv6 use them
without the IPv4 routing stack. It also adds CONFIG_IPV4=n stubs for
several FIB helpers in ip_fib.h and route.h.

[ ... ]

[Severity: Low]
Does this heading still describe the whole block? Only rtm_ipv4_policy
moved to fib_core.c. The other declarations under it are still defined in
net/ipv4/fib_frontend.c:

  ip_fib_init()
  fib_gw_from_via()
  fib_compute_spec_dst()
  fib_info_nh_uses_dev()
  fib_validate_source()
  fib_unmerge()

The reverse happens further down in the same header.
fib_nh_common_init() and fib_nh_common_release() are now defined in
net/core/fib_core.c, but their declarations are still under the older
heading:

/* Exported by fib_semantics.c */
...
int fib_nh_common_init(struct net *net, struct fib_nh_common *nhc,
...
void fib_nh_common_release(struct fib_nh_common *nhc);

Could rtm_ipv4_policy get its own fib_core.c heading, with the
fib_nh_common_* declarations moved under it? As written, ip_fib_init() or
fib_validate_source() could look available when CONFIG_IPV4=n. The
headings are still like this at the end of the series.

[ ... ]

[Severity: Low]
Should fib_core.o be gated on CONFIG_INET? As written, it is built into
every CONFIG_NET kernel, including CONFIG_INET=n ones.

Everything in fib_core.c is INET specific. That covers the fnhe and
rtable cache teardown, rtm_ipv4_policy, ip_valid_fib_dump_req(),
fib_nexthop_info() and fib_add_nexthop(). The callers are in net/ipv4 and
net/ipv6, and both of those need INET.

The only caller outside INET is net/mpls/af_mpls.c, and it already
skips ip_valid_fib_dump_req() when INET is off:

#if IS_ENABLED(CONFIG_INET)
static int mpls_valid_fib_dump_req(...)
{
	return ip_valid_fib_dump_req(net, nlh, filter, cb);
}
#else

So on !INET kernels this is unused code plus an unused
EXPORT_SYMBOL_GPL(ip_valid_fib_dump_req). The same Makefile already gates
an INET-only object this way:

ifdef CONFIG_INET
obj-$(CONFIG_BPF_SYSCALL) += bpf_ksock.o
endif

This does not break the !INET build today. The rule is still
unconditional at the end of the series.

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