Thread (4 messages) 4 messages, 2 authors, 2026-03-31

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

From: Fernando Fernandez Mancera <hidden>
Date: 2026-03-31 10:45:49

On 3/31/26 12:36 PM, Paolo Abeni wrote:
On 3/27/26 1:02 PM, Fernando Fernandez Mancera wrote:
quoted
When IPv4 device configuration parameters are updated via netlink, the
kernel currently only updates the value. This bypasses several
post-modification actions that occur when these same parameters are
updated via sysctl, such as flushing the routing cache or emitting
RTM_NEWNETCONF notifications.

This patch addresses the inconsistency by calling the
devinet_conf_post_set() helper inside inet_set_link_af(). If a flush is
required, we defer it until the netlink attribute parsing loop
completes.
IMHO the above deserve some additional self-test triggering the relevant
code.
Hi Paolo,

sure, I could include a selftest for this.
quoted
This ensures consistent behavior and side-effects for devconf changes,
regardless of whether they are initiated via sysctl or netlink.

Signed-off-by: Fernando Fernandez Mancera <redacted>
---
v2: handled forwarding notification and disabling LRO
---
  net/ipv4/devinet.c | 29 +++++++++++++++++++++++++++--
  1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 8300516fb38f..a35b72662e43 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,
  					    NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
  					    ifindex, cnf);
  		break;
+	case IPV4_DEVCONF_FORWARDING:
+		if (new == 1) {
AI reviews says:

Does this check miss cases where forwarding is enabled with a value other
than 1?
The sysctl path allows enabling IP forwarding with any non-zero value. If
a user sets IPV4_DEVCONF_FORWARDING to 2 via netlink, forwarding will be
enabled but this check is bypassed, leaving Large Receive Offload (LRO)
enabled. Could this cause LRO-coalesced packets to be forwarded without
proper software segmentation?
If a user sets IPV4_DEVCONF_FORWARDING to 2 via netlink the value would 
be rejected. There is netlink attribute validation and FORWARDING is set 
as boolean with the range [0, 1].

This cannot happen.
quoted
+			/* 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);
AI review says:

Is it safe to call netif_disable_lro() here without holding the device
operations lock?
While inet_set_link_af() runs under rtnl_lock(), netif_disable_lro()
modifies device features and calls netdev_update_features(), which
asserts that the device operations lock is held. Calling this without
the lock might trigger an assertion or expose the device to data races
with concurrent feature updates or BPF/XDP attachments.
Should this use dev_disable_lro() instead, which acquires the required
lock before disabling LRO?
It is not possible to call this without the holding the lock. The device 
operations lock is held when handling IFLA_AF_SPEC at 
net/core/rtnetlink.c. Calling netif_disable_lro() is the right thing to 
do IMHO. Keep on mind that FORWARDING case here is only triggered by 
netlink path and not sysctl.

Thanks,
Fernando.
/P
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help