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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Syntax error in `nh_netdev_event` when `CONFIG_IPV4` is enabled, causing a build failure.
- [Low] Parameter name mismatch in the dummy inline function for `fib_nh_init`.
--

commit 243925357332d15aa13a077472006e05019dde8d
Author: Fernando Fernandez Mancera <fmancera@suse.de>

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

This commit creates fib_core.c and moves functions for common nexthop
initialization and release from fib_semantics.c to fib_core.c. This makes
shared FIB and nexthop logic accessible to IPv6 when the IPv4 routing
stack is omitted.

> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 0a35355fb0f33..4bbafb7b723f2 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
[ ... ]
> @@ -578,10 +584,23 @@ void fib_select_multipath(struct fib_result *res, int hash,
>  void fib_select_path(struct net *net, struct fib_result *res,
>  		     struct flowi4 *fl4, const struct sk_buff *skb);
>  
> +#if IS_ENABLED(CONFIG_IPV4)
>  int fib_nh_init(struct net *net, struct fib_nh *fib_nh,
>  		struct fib_config *cfg, int nh_weight,
>  		struct netlink_ext_ack *extack);
>  void fib_nh_release(struct net *net, struct fib_nh *fib_nh);
> +#else
> +static inline int fib_nh_init(struct net *net, struct fib_nh *fib_nh,
> +			      struct fib_config *cfg, int gfp,
                                                              ^^^

[Severity: Low]
Does this code use the correct parameter name here? The actual definition of
fib_nh_init in the IPv4 block uses int nh_weight for the fourth parameter.
Using gfp in the stub obscures the API contract and might mislead developers
into passing a GFP flag instead of a routing weight.

> +			      struct netlink_ext_ack *extack)
> +{
> +	return -EAFNOSUPPORT;
> +}
[ ... ]
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index a7c2b8dced4e7..ea747fc2223c4 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
[ ... ]
> @@ -4004,8 +4005,12 @@ static int nh_netdev_event(struct notifier_block *this,
>  			nexthop_flush_dev(dev, event);
>  		break;
>  	case NETDEV_CHANGEMTU:
> +#if IS_ENABLED(CONFIG_IPV4)
> +		struct netdev_notifier_info_ext *info_ext;
> +
>  		info_ext = ptr;
>  		nexthop_sync_mtu(dev, info_ext->ext.mtu);
> +#endif

[Severity: Low]
Can this sequence cause a build failure when CONFIG_IPV4 is enabled? In C, a
case label cannot be immediately followed by a declaration. This causes a
compilation error because a label can only be part of a statement, and a
declaration is not a statement. Could we add a block or move the declaration
to avoid breaking the build?

>  		rt_cache_flush(dev_net(dev));
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910144914.8025-1-fmancera@suse.de?part=9