Re: [PATCH 2/3 net-next v6] ipv4: handle devconf post-set actions on netlink updates

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ipv4: handle devconf post-set actions on netlink updates

This commit ensures that IPv4 device configuration parameters updated
via netlink trigger the same post-modification actions as sysctl updates.
It addresses the inconsistency by calling devinet_conf_post_set() inside
inet_set_link_af() and deferring cache flushing.

> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 8300516fb38f4..a35b72662e431 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2161,6 +2161,20 @@ static bool devinet_conf_post_set(struct net *net, struct ipv4_devconf *cnf,
[ ... ]
> +	case IPV4_DEVCONF_FORWARDING:
> +		if (new == 1) {
> +			/* it is safe to use container_of() because forwarding case
> +			 * is only used by the netlink path
> +			 */
> +			struct in_device *idev = container_of(cnf, struct in_device, cnf);
> +
> +			netif_disable_lro(idev->dev);

[Severity: High]
Does calling netif_disable_lro() directly here without acquiring the
netdev_lock_ops mutex cause lockdep assertion failures?

Since inet_set_link_af() runs only under rtnl_lock() and does not acquire
the device ops lock, this direct invocation appears to bypass the locking
requirements for netif_disable_lro(), which modifies dev->wanted_features
and invokes netdev_update_features(dev).

On drivers that opt-in to netdev_lock_ops, does this create a concurrent
data race on the device features mask? Should this use dev_disable_lro()
instead, which correctly acquires and releases netdev_lock_ops(dev) around
the netif_disable_lro() call?

> +		}
> +
> +		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +					    NETCONFA_FORWARDING,
> +					    ifindex, cnf);
> +		return true;