[PATCH net v3] ipv4: bump rt_genid when a relevant devconf value changes through netlink

[PATCH net v3] ipv4: bump rt_genid when a relevant devconf value changes through netlink

[Fernando Fernandez Mancera]

When modifying IPv4 devconf values using netlink for some relevant
fields the rt_cache_flush() call was missing. In addition, if forwarding
is enabled on the interface then disable LRO.

This is needed to avoid possible connectivity issues and ease the
responsabilities of user space tools.

Fixes: 9f0f7272ac95 ("ipv4: AF_INET link address family")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: use netif_disable_lro() as we already hold netdev_need_ops_lock()
and use net_device struct passed as argument instead of using in_dev->dev
v3: separate it from the series and sent it to net tree instead
---
 net/ipv4/devinet.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 537bb6c315d2..5205fbe99963 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2098,6 +2098,8 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
 {
 	struct in_device *in_dev = __in_dev_get_rtnl(dev);
 	struct nlattr *a, *tb[IFLA_INET_MAX+1];
+	struct net *net = dev_net(dev);
+	bool flush_cache = false;
 	int rem;
 
 	if (!in_dev)
@@ -2107,8 +2109,27 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
 		return -EINVAL;
 
 	if (tb[IFLA_INET_CONF]) {
-		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
+		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
 			ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
+
+			switch (nla_type(a)) {
+			case IPV4_DEVCONF_FORWARDING:
+				if (nla_get_u32(a))
+					netif_disable_lro(dev);
+				fallthrough;
+			case IPV4_DEVCONF_NOXFRM:
+			case IPV4_DEVCONF_NOPOLICY:
+			case IPV4_DEVCONF_PROMOTE_SECONDARIES:
+			case IPV4_DEVCONF_ROUTE_LOCALNET:
+			case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
+				flush_cache = true;
+				break;
+			default:
+				break;
+			}
+		}
+		if (flush_cache)
+			rt_cache_flush(net);
 	}
 
 	return 0;
-- 
2.53.0

Re: [PATCH net v3] ipv4: bump rt_genid when a relevant devconf value changes through netlink

[Jakub Kicinski]

On Mon,  2 Mar 2026 14:08:57 +0100 Fernando Fernandez Mancera wrote:
> +			switch (nla_type(a)) {
> +			case IPV4_DEVCONF_FORWARDING:
> +				if (nla_get_u32(a))
> +					netif_disable_lro(dev);

why not dev_disable_lro()??

> +				fallthrough;
> +			case IPV4_DEVCONF_NOXFRM:
> +			case IPV4_DEVCONF_NOPOLICY:
> +			case IPV4_DEVCONF_PROMOTE_SECONDARIES:
> +			case IPV4_DEVCONF_ROUTE_LOCALNET:
> +			case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
> +				flush_cache = true;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +		if (flush_cache)
> +			rt_cache_flush(net);
>  	}

Looking closer at this I'm struggling to connect this to
devinet_conf_proc(). The attrs you're snooping here are completely different!?
Logic for when rt_cache_flush() happens is different.

inet_netconf_notify_devconf() is missing even after the patch.
-- 
pw-bot: cr

Re: [PATCH net v3] ipv4: bump rt_genid when a relevant devconf value changes through netlink

[Fernando Fernandez Mancera]

On 3/4/26 3:38 AM, Jakub Kicinski wrote:
> On Mon,  2 Mar 2026 14:08:57 +0100 Fernando Fernandez Mancera wrote:
>> +			switch (nla_type(a)) {
>> +			case IPV4_DEVCONF_FORWARDING:
>> +				if (nla_get_u32(a))
>> +					netif_disable_lro(dev);
> 
> why not dev_disable_lro()??

dev_disable_lro() takes netdev_lock_ops() but when handling IFLA_AF_SPEC 
at net/core/rtnetlink.c:3320 we are already holding netdev lock ops. So 
we need to call netif_disable_lro() instead.

> 
>> +				fallthrough;
>> +			case IPV4_DEVCONF_NOXFRM:
>> +			case IPV4_DEVCONF_NOPOLICY:
>> +			case IPV4_DEVCONF_PROMOTE_SECONDARIES:
>> +			case IPV4_DEVCONF_ROUTE_LOCALNET:
>> +			case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
>> +				flush_cache = true;
>> +				break;
>> +			default:
>> +				break;
>> +			}
>> +		}
>> +		if (flush_cache)
>> +			rt_cache_flush(net);
>>   	}
> 
> Looking closer at this I'm struggling to connect this to
> devinet_conf_proc(). The attrs you're snooping here are completely different!?
> Logic for when rt_cache_flush() happens is different.
> 

So I was planning to to handle inet_netconf_notify_devconf() calls in a 
follow-up patch to net-next because it might require some more changes 
and also the missing notification shouldn't be net tree material IMHO.

This is handling ipv4_doint_and_flush() which is used by 
DEVINET_SYSCTL_FLUSHING_ENTRY() macro. But now that you mention it, 
BC_FORWARDING and ACCEPT_LOCAL are part of devinet_conf_proc() while 
they probably could go to ipv4_doint_and_flush().

In addition, I guess we should flush the cache only if the value 
changed. I will add that bit too.

P.S: there is also dead code in devinet_conf_proc() as `i == 
IPV4_DEVCONF_ROUTE_LOCALNET - 1` won't never evaluate as true.

Thanks,
Fernando.

> inet_netconf_notify_devconf() is missing even after the patch.

Re: [PATCH net v3] ipv4: bump rt_genid when a relevant devconf value changes through netlink

[Jakub Kicinski]

On Wed, 4 Mar 2026 10:03:37 +0100 Fernando Fernandez Mancera wrote:
> On 3/4/26 3:38 AM, Jakub Kicinski wrote:
> > On Mon,  2 Mar 2026 14:08:57 +0100 Fernando Fernandez Mancera wrote:  
> >> +			switch (nla_type(a)) {
> >> +			case IPV4_DEVCONF_FORWARDING:
> >> +				if (nla_get_u32(a))
> >> +					netif_disable_lro(dev);  
> > 
> > why not dev_disable_lro()??  
> 
> dev_disable_lro() takes netdev_lock_ops() but when handling IFLA_AF_SPEC 
> at net/core/rtnetlink.c:3320 we are already holding netdev lock ops. So 
> we need to call netif_disable_lro() instead.

Makes sense

> >> +				fallthrough;
> >> +			case IPV4_DEVCONF_NOXFRM:
> >> +			case IPV4_DEVCONF_NOPOLICY:
> >> +			case IPV4_DEVCONF_PROMOTE_SECONDARIES:
> >> +			case IPV4_DEVCONF_ROUTE_LOCALNET:
> >> +			case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
> >> +				flush_cache = true;
> >> +				break;
> >> +			default:
> >> +				break;
> >> +			}
> >> +		}
> >> +		if (flush_cache)
> >> +			rt_cache_flush(net);
> >>   	}  
> > 
> > Looking closer at this I'm struggling to connect this to
> > devinet_conf_proc(). The attrs you're snooping here are completely different!?
> > Logic for when rt_cache_flush() happens is different.
> 
> So I was planning to to handle inet_netconf_notify_devconf() calls in a 
> follow-up patch to net-next because it might require some more changes 
> and also the missing notification shouldn't be net tree material IMHO.

Fair, let's call it out in the commit message, tho

> This is handling ipv4_doint_and_flush() which is used by 
> DEVINET_SYSCTL_FLUSHING_ENTRY() macro. But now that you mention it, 
> BC_FORWARDING and ACCEPT_LOCAL are part of devinet_conf_proc() while 
> they probably could go to ipv4_doint_and_flush().
> 
> In addition, I guess we should flush the cache only if the value 
> changed. I will add that bit too.

Thanks!