[PATCH net-next 0/3] net: rmnet: lockless rmnet_fill_info()

[PATCH net-next 0/3] net: rmnet: lockless rmnet_fill_info()

[Eric Dumazet]

rmnet_fill_info() currently relies on RTNL being held, because it
uses rmnet_get_port_rtnl() to reach the rmnet_port attached to the
underlying real device.

While auditing the fields exposed there, it appears that both
priv->mux_id and port->data_format are written under RTNL but read
from the data path without any lock. Note that rmnet_newlink() can
update port->data_format of an already active port, and that
rmnet_changelink() can change both fields while traffic is flowing.

- Patch 1 annotates the data-races around port->data_format. It also
  samples the field only once per packet and passes the value down,
  so that both ends of a packet transformation agree on the format
  (TX used to size the MAP headroom and set MAP_NEXT_HEADER_FLAG
  from two separate reads), and prevents rmnet_changelink() from
  publishing an intermediate value to the data path.

- Patch 2 annotates the data-races around priv->mux_id and
  ep->mux_id. It also publishes ep->mux_id before the endpoint is
  inserted into its new bucket, so that a concurrent lookup walking
  that bucket can no longer miss it and drop the packet.

- Patch 3 converts rmnet_fill_info() to RCU. rmnet_get_port_rcu()
  was only called from the data path and thus used
  rcu_dereference_bh(); its lockdep condition is relaxed so that it
  can also be called from process context.

The lockless lookup is safe because rmnet_get_port_rcu() checks
real_dev->rx_handler before returning rx_handler_data, while
rmnet_unregister_real_device() clears rx_handler and waits for a
grace period (in netdev_rx_handler_unregister()) before freeing the
port.

This is part of a larger effort to remove the RTNL dependency from
rtnl_link_ops->fill_info().

Assisted-by: LLM
Cc: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>

Eric Dumazet (3):
  net: rmnet: annotate data-races around port->data_format
  net: rmnet: annotate data-races around mux_id
  net: rmnet: no longer rely on RTNL in rmnet_fill_info()

 .../ethernet/qualcomm/rmnet/rmnet_config.c    | 54 +++++++++----------
 .../ethernet/qualcomm/rmnet/rmnet_config.h    |  2 +-
 .../ethernet/qualcomm/rmnet/rmnet_handlers.c  | 34 +++++++-----
 .../net/ethernet/qualcomm/rmnet/rmnet_map.h   |  9 ++--
 .../qualcomm/rmnet/rmnet_map_command.c        |  9 ++--
 .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 14 ++---
 .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   |  2 +-
 7 files changed, 65 insertions(+), 59 deletions(-)

-- 
2.55.0.1032.g73a4cd73de-goog

[PATCH net-next 1/3] net: rmnet: annotate data-races around port->data_format

[Eric Dumazet]

port->data_format is written under RTNL from rmnet_newlink() and
rmnet_changelink(), but it is read from the data path without any
lock, both from RX (rmnet_rx_handler() context) and TX
(rmnet_egress_handler() context).

Note that rmnet_newlink() can be called for a real_dev which is
already hooked to rmnet: the rx_handler is live and traffic can
already be flowing when port->data_format is overwritten.

Add the missing READ_ONCE()/WRITE_ONCE() annotations.

While at it, sample port->data_format only once per packet and pass
the value down, so that all the decisions taken for a given packet
are based on a single consistent value.

Otherwise the two ends of a packet transformation could disagree.
On TX, rmnet_map_egress_handler() sized the headroom from one read
while rmnet_map_add_map_header() decided on MAP_NEXT_HEADER_FLAG
from another one, so a concurrent rmnet_changelink() could produce
a MAP header announcing a v5 csum header that was neither reserved
nor written. On RX, rmnet_map_validate_packet_len() and
__rmnet_map_ingress_handler() could likewise disagree on the
expected layout, and rmnet_map_send_ack() could trim a dl csum
trailer that the ingress path never accounted for.

rmnet_map_add_map_header(), rmnet_map_command(),
rmnet_map_deaggregate(), rmnet_map_send_ack(),
rmnet_map_validate_packet_len() and __rmnet_map_ingress_handler()
therefore get the value from their caller instead of re-reading it.

rmnet_changelink() now computes the new value in a local variable
and publishes it with a single store, instead of letting the data
path observe the intermediate (old_data_format & ~flags->mask)
value.

rmnet_vnd_headroom() is left alone, all its callers hold RTNL.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>
---
 .../ethernet/qualcomm/rmnet/rmnet_config.c    | 10 +++---
 .../ethernet/qualcomm/rmnet/rmnet_handlers.c  | 32 +++++++++++--------
 .../net/ethernet/qualcomm/rmnet/rmnet_map.h   |  9 +++---
 .../qualcomm/rmnet/rmnet_map_command.c        |  9 +++---
 .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 14 ++++----
 5 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index bed6f63facf250bd1f8d07d09f7715918421a88d..59ef8b4ce5321ebbd1d416fd9da0a820e53ecf9e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -176,7 +176,7 @@ static int rmnet_newlink(struct net_device *dev,
 	}
 
 	netdev_dbg(dev, "data format [0x%08X]\n", data_format);
-	port->data_format = data_format;
+	WRITE_ONCE(port->data_format, data_format);
 
 	return 0;
 
@@ -342,14 +342,16 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (data[IFLA_RMNET_FLAGS]) {
 		struct ifla_rmnet_flags *flags;
 		u32 old_data_format;
+		u32 data_format;
 
 		old_data_format = port->data_format;
 		flags = nla_data(data[IFLA_RMNET_FLAGS]);
-		port->data_format &= ~flags->mask;
-		port->data_format |= flags->flags & flags->mask;
+		data_format = old_data_format & ~flags->mask;
+		data_format |= flags->flags & flags->mask;
+		WRITE_ONCE(port->data_format, data_format);
 
 		if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
-			port->data_format = old_data_format;
+			WRITE_ONCE(port->data_format, old_data_format);
 			NL_SET_ERR_MSG_MOD(extack, "Invalid MTU on real dev");
 			return -EINVAL;
 		}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index d055a2628d8c9d0b6e7e85eb98f6fa9b5a1bd531..d4fec2fff227cd93f14eb1802aff185e6d496786 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -54,7 +54,8 @@ rmnet_deliver_skb(struct sk_buff *skb)
 
 static void
 __rmnet_map_ingress_handler(struct sk_buff *skb,
-			    struct rmnet_port *port)
+			    struct rmnet_port *port,
+			    u32 data_format)
 {
 	struct rmnet_map_header *map_header = (void *)skb->data;
 	struct rmnet_endpoint *ep;
@@ -63,8 +64,8 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
 
 	if (map_header->flags & MAP_CMD_FLAG) {
 		/* Packet contains a MAP command (not data) */
-		if (port->data_format & RMNET_FLAGS_INGRESS_MAP_COMMANDS)
-			return rmnet_map_command(skb, port);
+		if (data_format & RMNET_FLAGS_INGRESS_MAP_COMMANDS)
+			return rmnet_map_command(skb, port, data_format);
 
 		goto free_skb;
 	}
@@ -82,7 +83,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
 
 	skb->dev = ep->egress_dev;
 
-	if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
+	if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
 	    (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
 		if (rmnet_map_process_next_hdr_packet(skb, len))
 			goto free_skb;
@@ -92,7 +93,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
 		/* Subtract MAP header */
 		skb_pull(skb, sizeof(*map_header));
 		rmnet_set_skb_proto(skb);
-		if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4 &&
+		if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4 &&
 		    !rmnet_map_checksum_downlink_packet(skb, len + pad))
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
@@ -110,6 +111,7 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
 			  struct rmnet_port *port)
 {
 	struct sk_buff *skbn;
+	u32 data_format;
 
 	if (skb->dev->type == ARPHRD_ETHER) {
 		if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_ATOMIC)) {
@@ -120,14 +122,16 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
 		skb_push(skb, ETH_HLEN);
 	}
 
-	if (port->data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
-		while ((skbn = rmnet_map_deaggregate(skb, port)) != NULL)
-			__rmnet_map_ingress_handler(skbn, port);
+	data_format = READ_ONCE(port->data_format);
+
+	if (data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
+		while ((skbn = rmnet_map_deaggregate(skb, data_format)) != NULL)
+			__rmnet_map_ingress_handler(skbn, port, data_format);
 
 		consume_skb(skb);
 	} else {
-		if (rmnet_map_validate_packet_len(skb, port))
-			__rmnet_map_ingress_handler(skb, port);
+		if (rmnet_map_validate_packet_len(skb, data_format))
+			__rmnet_map_ingress_handler(skb, port, data_format);
 		else
 			kfree_skb(skb);
 	}
@@ -139,14 +143,16 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 {
 	int required_headroom, additional_header_len, csum_type = 0;
 	struct rmnet_map_header *map_header;
+	u32 data_format;
 
 	additional_header_len = 0;
 	required_headroom = sizeof(struct rmnet_map_header);
 
-	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
+	data_format = READ_ONCE(port->data_format);
+	if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
 		additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
 		csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
-	} else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
+	} else if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
 		additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
 		csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
 	}
@@ -161,7 +167,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 						 csum_type);
 
 	map_header = rmnet_map_add_map_header(skb, additional_header_len,
-					      port, 0);
+					      data_format, 0);
 	if (!map_header)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 60ca8b780c88ad7d9a6334370c0ff73a83e02d2d..0977e495f5915539fc1154bb592a2648aef43cda 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -41,12 +41,13 @@ enum rmnet_map_commands {
 #define RMNET_MAP_ADD_PAD_BYTES       1
 
 struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
-				      struct rmnet_port *port);
+				      u32 data_format);
 struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 						  int hdrlen,
-						  struct rmnet_port *port,
+						  u32 data_format,
 						  int pad);
-void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
+void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port,
+		       u32 data_format);
 int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
 void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
 				      struct rmnet_port *port,
@@ -59,6 +60,6 @@ void rmnet_map_tx_aggregate_init(struct rmnet_port *port);
 void rmnet_map_tx_aggregate_exit(struct rmnet_port *port);
 void rmnet_map_update_ul_agg_config(struct rmnet_port *port, u32 size,
 				    u32 count, u32 time);
-u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port);
+u32 rmnet_map_validate_packet_len(struct sk_buff *skb, u32 data_format);
 
 #endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
index add0f5ade2e6174427abff532d160f89122059f7..d334727e1f5256201996fc56c419ec05f04dde4e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
@@ -48,13 +48,13 @@ static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
 
 static void rmnet_map_send_ack(struct sk_buff *skb,
 			       unsigned char type,
-			       struct rmnet_port *port)
+			       u32 data_format)
 {
 	struct rmnet_map_header *map_header = (void *)skb->data;
 	struct rmnet_map_control_command *cmd;
 	struct net_device *dev = skb->dev;
 
-	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
+	if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
 		skb_trim(skb,
 			 skb->len - sizeof(struct rmnet_map_dl_csum_trailer));
 
@@ -72,7 +72,8 @@ static void rmnet_map_send_ack(struct sk_buff *skb,
 /* Process MAP command frame and send N/ACK message as appropriate. Message cmd
  * name is decoded here and appropriate handler is called.
  */
-void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
+void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port,
+		       u32 data_format)
 {
 	struct rmnet_map_header *map_header = (void *)skb->data;
 	struct rmnet_map_control_command *cmd;
@@ -98,5 +99,5 @@ void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
 		break;
 	}
 	if (rc == RMNET_MAP_COMMAND_ACK)
-		rmnet_map_send_ack(skb, rc, port);
+		rmnet_map_send_ack(skb, rc, data_format);
 }
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index e6f48dc9fe64d7e563a0ba355c1ede35c73b4593..39d6d084e73f00f9fcd475ad98b16dde4cb266c6 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -294,7 +294,7 @@ static void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
  */
 struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 						  int hdrlen,
-						  struct rmnet_port *port,
+						  u32 data_format,
 						  int pad)
 {
 	struct rmnet_map_header *map_header;
@@ -306,7 +306,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 	memset(map_header, 0, sizeof(struct rmnet_map_header));
 
 	/* Set next_hdr bit for csum offload packets */
-	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
+	if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
 		map_header->flags |= MAP_NEXT_HEADER_FLAG;
 
 	if (pad == RMNET_MAP_NO_PAD_BYTES) {
@@ -333,7 +333,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 	return map_header;
 }
 
-u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
+u32 rmnet_map_validate_packet_len(struct sk_buff *skb, u32 data_format)
 {
 	struct rmnet_map_v5_csum_header *next_hdr = NULL;
 	struct rmnet_map_header *maph;
@@ -351,9 +351,9 @@ u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
 
 	packet_len = ntohs(maph->pkt_len) + sizeof(*maph);
 
-	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
+	if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
 		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
-	} else if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
+	} else if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
 		   !(maph->flags & MAP_CMD_FLAG)) {
 		/* Mapv5 data pkt without csum hdr is invalid */
 		if (!(maph->flags & MAP_NEXT_HEADER_FLAG))
@@ -381,12 +381,12 @@ u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
  * is responsible for freeing the original skb.
  */
 struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
-				      struct rmnet_port *port)
+				      u32 data_format)
 {
 	struct sk_buff *skbn;
 	u32 packet_len;
 
-	packet_len = rmnet_map_validate_packet_len(skb, port);
+	packet_len = rmnet_map_validate_packet_len(skb, data_format);
 	if (!packet_len)
 		return NULL;
 
-- 
2.55.0.1032.g73a4cd73de-goog

[PATCH net-next 2/3] net: rmnet: annotate data-races around mux_id

[Eric Dumazet]

priv->mux_id is written under RTNL from rmnet_vnd_newlink() and
rmnet_changelink(), but rmnet_egress_handler() reads it from the
transmit path without any lock.

Likewise ep->mux_id is written under RTNL from rmnet_changelink(),
while rmnet_get_endpoint() reads it from the receive path under
RCU only.

Add the missing READ_ONCE()/WRITE_ONCE() annotations.

While at it, publish ep->mux_id before inserting the endpoint in
its new bucket in rmnet_changelink(). Otherwise a concurrent
rmnet_get_endpoint() walking the new bucket could find the
endpoint still carrying its old mux_id, and drop the packet.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c   | 6 +++---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 2 +-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 59ef8b4ce5321ebbd1d416fd9da0a820e53ecf9e..248a9d822409b7cbf3739c085487644b7025dd37 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -331,11 +331,11 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
 			}
 
 			hlist_del_init_rcu(&ep->hlnode);
+			WRITE_ONCE(ep->mux_id, mux_id);
 			hlist_add_head_rcu(&ep->hlnode,
 					   &port->muxed_ep[mux_id]);
 
-			ep->mux_id = mux_id;
-			priv->mux_id = mux_id;
+			WRITE_ONCE(priv->mux_id, mux_id);
 		}
 	}
 
@@ -427,7 +427,7 @@ struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id)
 
 	hlist_for_each_entry_rcu(ep, &port->muxed_ep[mux_id], hlnode,
 				 lockdep_rtnl_is_held()) {
-		if (ep->mux_id == mux_id)
+		if (READ_ONCE(ep->mux_id) == mux_id)
 			return ep;
 	}
 
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index d4fec2fff227cd93f14eb1802aff185e6d496786..aa5523f4618eb350ec3da20b7f085d955b3403a2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -261,7 +261,7 @@ void rmnet_egress_handler(struct sk_buff *skb)
 	orig_dev = skb->dev;
 	priv = netdev_priv(orig_dev);
 	skb->dev = priv->real_dev;
-	mux_id = priv->mux_id;
+	mux_id = READ_ONCE(priv->mux_id);
 
 	port = rmnet_get_port_rcu(skb->dev);
 	if (!port)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 4f0ddcedfa9790dc5843fdea85b2da4bc8329775..9594c97c918565e93ac70ac73340f705b391bd18 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -332,7 +332,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 
 		rmnet_dev->rtnl_link_ops = &rmnet_link_ops;
 
-		priv->mux_id = id;
+		WRITE_ONCE(priv->mux_id, id);
 
 		netdev_dbg(rmnet_dev, "rmnet dev created\n");
 	}
-- 
2.55.0.1032.g73a4cd73de-goog

[PATCH net-next 3/3] net: rmnet: no longer rely on RTNL in rmnet_fill_info()

[Eric Dumazet]

rmnet_fill_info() used rmnet_get_port_rtnl(), and thus required
RTNL.

Now that priv->mux_id and port->data_format are properly
annotated, rmnet_fill_info() can simply fetch the port under
rcu_read_lock().

rmnet_get_port_rcu() was only used from the data path and thus
used rcu_dereference_bh(). Relax its lockdep condition so that it
can also be called from process context under a plain
rcu_read_lock(), and use it from rmnet_fill_info().

Note that rmnet_get_port_rcu() checks real_dev->rx_handler before
returning rx_handler_data: this is what makes the lockless lookup
safe against rmnet_unregister_real_device(), which clears
rx_handler, waits for a grace period in
netdev_rx_handler_unregister(), and only then frees the port.

While at it, add missing const qualifiers.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>
---
 .../ethernet/qualcomm/rmnet/rmnet_config.c    | 38 +++++++++----------
 .../ethernet/qualcomm/rmnet/rmnet_config.h    |  2 +-
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 248a9d822409b7cbf3739c085487644b7025dd37..b930f638ec448de839806852a74fc4b2f8888e43 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -371,32 +371,24 @@ static size_t rmnet_get_size(const struct net_device *dev)
 
 static int rmnet_fill_info(struct sk_buff *skb, const struct net_device *dev)
 {
-	struct rmnet_priv *priv = netdev_priv(dev);
-	struct net_device *real_dev;
+	const struct rmnet_priv *priv = netdev_priv(dev);
+	const struct rmnet_port *port;
 	struct ifla_rmnet_flags f;
-	struct rmnet_port *port;
 
-	real_dev = priv->real_dev;
+	if (nla_put_u16(skb, IFLA_RMNET_MUX_ID, READ_ONCE(priv->mux_id)))
+		return -EMSGSIZE;
 
-	if (nla_put_u16(skb, IFLA_RMNET_MUX_ID, priv->mux_id))
-		goto nla_put_failure;
-
-	if (rmnet_is_real_dev_registered(real_dev)) {
-		port = rmnet_get_port_rtnl(real_dev);
-		f.flags = port->data_format;
-	} else {
-		f.flags = 0;
-	}
+	rcu_read_lock();
+	port = rmnet_get_port_rcu(priv->real_dev);
+	f.flags = port ? READ_ONCE(port->data_format) : 0;
+	rcu_read_unlock();
 
 	f.mask  = ~0;
 
 	if (nla_put(skb, IFLA_RMNET_FLAGS, sizeof(f), &f))
-		goto nla_put_failure;
+		return -EMSGSIZE;
 
 	return 0;
-
-nla_put_failure:
-	return -EMSGSIZE;
 }
 
 struct rtnl_link_ops rmnet_link_ops __read_mostly = {
@@ -413,12 +405,16 @@ struct rtnl_link_ops rmnet_link_ops __read_mostly = {
 	.fill_info	= rmnet_fill_info,
 };
 
-struct rmnet_port *rmnet_get_port_rcu(struct net_device *real_dev)
+/* Can be called from a RCU read-side critical section, with or
+ * without BH disabled.
+ */
+struct rmnet_port *rmnet_get_port_rcu(const struct net_device *real_dev)
 {
-	if (rmnet_is_real_dev_registered(real_dev))
-		return rcu_dereference_bh(real_dev->rx_handler_data);
-	else
+	if (!rmnet_is_real_dev_registered(real_dev))
 		return NULL;
+
+	return rcu_dereference_check(real_dev->rx_handler_data,
+				     rcu_read_lock_bh_held());
 }
 
 struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index f50fae1c6bdd5d4edfb18476e2ca44057921a472..5adda0323dda776d69d50d22bc75456b43442e3d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -90,7 +90,7 @@ struct rmnet_priv {
 	struct rmnet_priv_stats stats;
 };
 
-struct rmnet_port *rmnet_get_port_rcu(struct net_device *real_dev);
+struct rmnet_port *rmnet_get_port_rcu(const struct net_device *real_dev);
 struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id);
 int rmnet_add_bridge(struct net_device *rmnet_dev,
 		     struct net_device *slave_dev,
-- 
2.55.0.1032.g73a4cd73de-goog

Re: [PATCH net-next 1/3] net: rmnet: annotate data-races around port->data_format

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 (2) · Low: 1

- [Medium] Even with a single per-packet snapshot of port->data_format,
  the individual stages still resolve the checksum-version bits…
- [Low] The commit message of "net: rmnet: annotate data-races around
  port->data_format" documents concrete, user-visible wire-format…

Pre-existing issues:
- [Medium] port->rmnet_mode and port->bridge_ep are written with plain
  stores under RTNL (rmnet_newlink() at rmnet_config.c:163-164,…
- [Medium] rmnet_register_real_device() publishes the zero-initialized
  struct rmnet_port through netdev_rx_handler_register()…

--

net: rmnet: annotate data-races around port->data_format

This adds READ_ONCE()/WRITE_ONCE() annotations around
port->data_format, which is written under RTNL from rmnet_newlink() and
rmnet_changelink() but read from the RX and TX data paths without any
lock. The value is now sampled once per packet in
rmnet_map_ingress_handler() and rmnet_map_egress_handler() and passed
down to the MAP helpers instead of being re-read by each of them.

A couple of questions below about the commit message and about the
remaining asymmetry between the stages that consume the sampled value.

[Severity: Low]
The changelog describes concrete wire-format failures rather than just a
KCSAN annotation cleanup:

> so a concurrent rmnet_changelink() could produce
> a MAP header announcing a v5 csum header that was neither reserved
> nor written.

and

> rmnet_map_send_ack() could trim a dl csum
> trailer that the ingress path never accounted for.

Should this carry a Fixes: tag then?  Candidates would be
b6e5d27e32ef60 ("net: ethernet: rmnet: Add support for MAPv5 egress
packets"), which added the second read of port->data_format in
rmnet_map_add_map_header(), and 14452ca3b5ce ("net: qualcomm: rmnet:
Export mux_id and flags to netlink"), which made the flags runtime
configurable.

Without a tag, stable tooling will not pick this up.  If the intent is
net-next hardening instead (the third patch in the series,
"net: rmnet: no longer rely on RTNL in rmnet_fill_info()", is clearly a
refactor), would it be better to word the two paragraphs above as
theoretical rather than as observed packet corruption?

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index bed6f63facf25..59ef8b4ce5321 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c

[ ... ]

> @@ -342,14 +342,16 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (data[IFLA_RMNET_FLAGS]) {
>  		struct ifla_rmnet_flags *flags;
>  		u32 old_data_format;
> +		u32 data_format;
>  
>  		old_data_format = port->data_format;
>  		flags = nla_data(data[IFLA_RMNET_FLAGS]);
> -		port->data_format &= ~flags->mask;
> -		port->data_format |= flags->flags & flags->mask;
> +		data_format = old_data_format & ~flags->mask;
> +		data_format |= flags->flags & flags->mask;
> +		WRITE_ONCE(port->data_format, data_format);
>  
>  		if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
> -			port->data_format = old_data_format;
> +			WRITE_ONCE(port->data_format, old_data_format);
>  			NL_SET_ERR_MSG_MOD(extack, "Invalid MTU on real dev");
>  			return -EINVAL;
>  		}

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index d055a2628d8c9..d4fec2fff227c 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c

[ ... ]

> @@ -82,7 +83,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
>  
>  	skb->dev = ep->egress_dev;
>  
> -	if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
> +	if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
>  	    (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
>  		if (rmnet_map_process_next_hdr_packet(skb, len))
>  			goto free_skb;

[ ... ]

> @@ -120,14 +122,16 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
>  		skb_push(skb, ETH_HLEN);
>  	}
>  
> -	if (port->data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
> -		while ((skbn = rmnet_map_deaggregate(skb, port)) != NULL)
> -			__rmnet_map_ingress_handler(skbn, port);
> +	data_format = READ_ONCE(port->data_format);
> +
> +	if (data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
> +		while ((skbn = rmnet_map_deaggregate(skb, data_format)) != NULL)
> +			__rmnet_map_ingress_handler(skbn, port, data_format);
>  
>  		consume_skb(skb);
>  	} else {
> -		if (rmnet_map_validate_packet_len(skb, port))
> -			__rmnet_map_ingress_handler(skb, port);
> +		if (rmnet_map_validate_packet_len(skb, data_format))
> +			__rmnet_map_ingress_handler(skb, port, data_format);
>  		else
>  			kfree_skb(skb);
>  	}

[Severity: Medium]
This isn't a bug introduced by this patch, but the caller of
rmnet_map_ingress_handler() reads two more fields of the same struct
with plain loads while RTNL writers store them plainly:

drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c:rmnet_rx_handler() {
	switch (port->rmnet_mode) {
	case RMNET_EPMODE_VND:
		rmnet_map_ingress_handler(skb, port);
		break;
	case RMNET_EPMODE_BRIDGE:
		rmnet_bridge_handler(skb, port->bridge_ep);
		break;
	}

done:
	return RX_HANDLER_CONSUMED;
}

The writers update the pair in two separate stores, in opposite orders.
rmnet_add_bridge() does:

	port->rmnet_mode = RMNET_EPMODE_BRIDGE;
	port->bridge_ep = slave_dev;

while rmnet_unregister_bridge() does:

	port->bridge_ep = NULL;
	port->rmnet_mode = RMNET_EPMODE_VND;

Can a concurrent receive observe rmnet_mode == RMNET_EPMODE_BRIDGE
together with bridge_ep == NULL?  rmnet_bridge_handler() only acts
inside "if (bridge_dev)", yet rmnet_rx_handler() still returns
RX_HANDLER_CONSUMED, so does the skb leak in that window?

Since the RX path being annotated here is the same one that reads these
fields, would it make sense to annotate rmnet_mode and bridge_ep in the
same series?  They are still plain accesses at the end of the series
(94ca9c4e3244).

[Severity: Medium]
This is a pre-existing issue, but it is closely related to the note in
the changelog that "the rx_handler is live and traffic can already be
flowing".  rmnet_register_real_device() publishes the zero-filled port
through the rx_handler before any receive-visible state is set:

drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c:rmnet_register_real_device() {
	port = kzalloc_obj(*port);
	...
	rc = netdev_rx_handler_register(real_dev, rmnet_rx_handler, port);
	...
	for (entry = 0; entry < RMNET_MAX_LOGICAL_EP; entry++)
		INIT_HLIST_HEAD(&port->muxed_ep[entry]);
}

port->rmnet_mode is only assigned later, in rmnet_newlink(), after
rmnet_vnd_newlink() and netdev_upper_dev_link().  The valid values are
RMNET_EPMODE_VND (1) and RMNET_EPMODE_BRIDGE (2), so during that window
rmnet_mode is 0, the switch in rmnet_rx_handler() matches neither case
and there is no default, but RX_HANDLER_CONSUMED is still returned.

Is a packet arriving on the real_dev in that window leaked?  Would it be
better to install a valid rmnet_mode before netdev_rx_handler_register()?

> @@ -139,14 +143,16 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
>  {
>  	int required_headroom, additional_header_len, csum_type = 0;
>  	struct rmnet_map_header *map_header;
> +	u32 data_format;
>  
>  	additional_header_len = 0;
>  	required_headroom = sizeof(struct rmnet_map_header);
>  
> -	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
> +	data_format = READ_ONCE(port->data_format);
> +	if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
>  		additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
>  		csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
> -	} else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
> +	} else if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
>  		additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
>  		csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
>  	}
> @@ -161,7 +167,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
>  						 csum_type);
>  
>  	map_header = rmnet_map_add_map_header(skb, additional_header_len,
> -					      port, 0);
> +					      data_format, 0);
>  	if (!map_header)
>  		return -ENOMEM;

[Severity: Medium]
The snapshot removes the race, but the stages still resolve the
checksum-version bits differently, so is the symptom named in the
changelog ("a MAP header announcing a v5 csum header that was neither
reserved nor written") still reachable when both egress bits are set,
now without any race?

Nothing appears to reject that combination: rmnet_policy only checks the
size of struct ifla_rmnet_flags, rmnet_rtnl_validate() only validates
the MUX ID, and both rmnet_newlink() and rmnet_changelink() apply
flags->flags & flags->mask verbatim.  RMNET_FLAGS_EGRESS_MAP_CKSUMV4
and RMNET_FLAGS_EGRESS_MAP_CKSUMV5 are independent uAPI bits.

On TX, this function picks V4 first, so
rmnet_map_checksum_uplink_packet() -> rmnet_map_v4_checksum_uplink_packet()
pushes a v4 header:

	ul_header = (struct rmnet_map_ul_csum_header *)
		    skb_push(skb, sizeof(struct rmnet_map_ul_csum_header));

while rmnet_map_add_map_header() looks at the raw V5 bit of the same
sampled word:

	/* Set next_hdr bit for csum offload packets */
	if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
		map_header->flags |= MAP_NEXT_HEADER_FLAG;

Since the only caller passes pad == RMNET_MAP_NO_PAD_BYTES, the early
return keeps that flag.  Would passing the already-resolved csum_type
(or rejecting both version bits at configuration time) be preferable to
threading the raw bitmask, which only closes the race?

The same asymmetry exists on RX: rmnet_map_validate_packet_len() gives
INGRESS_MAP_CKSUMV4 precedence and adds 8 bytes of dl csum trailer,
__rmnet_map_ingress_handler() gives V5 precedence when
MAP_NEXT_HEADER_FLAG is set and consumes a 4-byte v5 header, and
rmnet_map_send_ack() trims 8 bytes purely on the V4 bit.  With both
ingress bits set, is a v5 frame rejected as 4 bytes short, and inside an
aggregate does rmnet_map_deaggregate() advance its cursor 4 bytes into
the following MAP header, breaking every later split point?

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
> index add0f5ade2e61..d334727e1f525 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
> @@ -48,13 +48,13 @@ static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
>  
>  static void rmnet_map_send_ack(struct sk_buff *skb,
>  			       unsigned char type,
> -			       struct rmnet_port *port)
> +			       u32 data_format)
>  {
>  	struct rmnet_map_header *map_header = (void *)skb->data;
>  	struct rmnet_map_control_command *cmd;
>  	struct net_device *dev = skb->dev;
>  
> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
> +	if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
>  		skb_trim(skb,
>  			 skb->len - sizeof(struct rmnet_map_dl_csum_trailer));
>  

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> index e6f48dc9fe64d..39d6d084e73f0 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -306,7 +306,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
>  	memset(map_header, 0, sizeof(struct rmnet_map_header));
>  
>  	/* Set next_hdr bit for csum offload packets */
> -	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
> +	if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
>  		map_header->flags |= MAP_NEXT_HEADER_FLAG;
>  
>  	if (pad == RMNET_MAP_NO_PAD_BYTES) {
> @@ -351,9 +351,9 @@ u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
>  
>  	packet_len = ntohs(maph->pkt_len) + sizeof(*maph);
>  
> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
> +	if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
>  		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
> -	} else if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
> +	} else if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
>  		   !(maph->flags & MAP_CMD_FLAG)) {
>  		/* Mapv5 data pkt without csum hdr is invalid */
>  		if (!(maph->flags & MAP_NEXT_HEADER_FLAG))

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916132231.1780091-1-edumazet%40google.com

Re: [PATCH net-next 2/3] net: rmnet: annotate data-races around mux_id

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The patch "net: rmnet: annotate data-races around mux_id"
  contains, besides the READ_ONCE()/WRITE_ONCE() annotations, a real…

--

net: rmnet: annotate data-races around mux_id

This adds READ_ONCE()/WRITE_ONCE() annotations around priv->mux_id and
ep->mux_id, which are written under RTNL but read from the transmit and
receive paths.  It also reorders rmnet_changelink() so that ep->mux_id is
published before the endpoint is inserted into its new hash bucket.

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 59ef8b4ce5321..248a9d822409b 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -331,11 +331,11 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
>  			}
>  
>  			hlist_del_init_rcu(&ep->hlnode);
> +			WRITE_ONCE(ep->mux_id, mux_id);
>  			hlist_add_head_rcu(&ep->hlnode,
>  					   &port->muxed_ep[mux_id]);
>  
> -			ep->mux_id = mux_id;
> -			priv->mux_id = mux_id;
> +			WRITE_ONCE(priv->mux_id, mux_id);

[Severity: Low]
Beyond the annotations, the reordering here looks like a functional fix
rather than a pure cleanup.  Should it carry a Fixes: tag?

With the old ordering the endpoint was visible in the new bucket while
still carrying its old mux_id, so a concurrent reader in
rmnet_get_endpoint() would reject it:

	hlist_for_each_entry_rcu(ep, &port->muxed_ep[mux_id], hlnode,
				 lockdep_rtnl_is_held()) {
		if (READ_ONCE(ep->mux_id) == mux_id)
			return ep;
	}

	return NULL;

and the RX path then frees the skb in __rmnet_map_ingress_handler():

	ep = rmnet_get_endpoint(port, mux_id);
	if (!ep)
		goto free_skb;

The reader is driven by inbound traffic in
rmnet_rx_handler() -> rmnet_map_ingress_handler() ->
__rmnet_map_ingress_handler(), while the writer side runs from
ip link set <rmnet dev> type rmnet mux_id N, so the observable effect is
occasional packet loss during a mux_id change.

git blame points at commit 2abb5792387e ("net: qualcomm: rmnet: Allow
configuration updates to existing devices") for the previous ordering, so
would the following trailer be appropriate to help stable pick this up?

	Fixes: 2abb5792387e ("net: qualcomm: rmnet: Allow configuration updates to existing devices")

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916132231.1780091-1-edumazet%40google.com

RE: [PATCH net-next 0/3] net: rmnet: lockless rmnet_fill_info()

[subash.a.kasiviswanathan]

> -----Original Message-----
> From: Eric Dumazet <edumazet@google.com>
> Sent: Wednesday, September 16, 2026 7:22 AM
> To: David S . Miller <davem@davemloft.net>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>; Kuniyuki Iwashima
> <kuniyu@google.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> netdev@vger.kernel.org; eric.dumazet@gmail.com; Eric Dumazet
> <edumazet@google.com>; Subash Abhinov Kasiviswanathan
> <subash.a.kasiviswanathan@oss.qualcomm.com>; Sean Tranchetti
> <sean.tranchetti@oss.qualcomm.com>
> Subject: [PATCH net-next 0/3] net: rmnet: lockless rmnet_fill_info()
> 
> rmnet_fill_info() currently relies on RTNL being held, because it uses
> rmnet_get_port_rtnl() to reach the rmnet_port attached to the underlying real
> device.
> 
> While auditing the fields exposed there, it appears that both
> priv->mux_id and port->data_format are written under RTNL but read
> from the data path without any lock. Note that rmnet_newlink() can update
> port->data_format of an already active port, and that
> rmnet_changelink() can change both fields while traffic is flowing.
> 
> - Patch 1 annotates the data-races around port->data_format. It also
>   samples the field only once per packet and passes the value down,
>   so that both ends of a packet transformation agree on the format
>   (TX used to size the MAP headroom and set MAP_NEXT_HEADER_FLAG
>   from two separate reads), and prevents rmnet_changelink() from
>   publishing an intermediate value to the data path.
> 
> - Patch 2 annotates the data-races around priv->mux_id and
>   ep->mux_id. It also publishes ep->mux_id before the endpoint is
>   inserted into its new bucket, so that a concurrent lookup walking
>   that bucket can no longer miss it and drop the packet.
> 
> - Patch 3 converts rmnet_fill_info() to RCU. rmnet_get_port_rcu()
>   was only called from the data path and thus used
>   rcu_dereference_bh(); its lockdep condition is relaxed so that it
>   can also be called from process context.
> 
> The lockless lookup is safe because rmnet_get_port_rcu() checks real_dev-
> >rx_handler before returning rx_handler_data, while
> rmnet_unregister_real_device() clears rx_handler and waits for a grace period
> (in netdev_rx_handler_unregister()) before freeing the port.
> 
> This is part of a larger effort to remove the RTNL dependency from
> rtnl_link_ops->fill_info().
> 
> Assisted-by: LLM
> Cc: Subash Abhinov Kasiviswanathan
> <subash.a.kasiviswanathan@oss.qualcomm.com>
> Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>
> 
> Eric Dumazet (3):
>   net: rmnet: annotate data-races around port->data_format
>   net: rmnet: annotate data-races around mux_id
>   net: rmnet: no longer rely on RTNL in rmnet_fill_info()
> 
>  .../ethernet/qualcomm/rmnet/rmnet_config.c    | 54 +++++++++----------
>  .../ethernet/qualcomm/rmnet/rmnet_config.h    |  2 +-
>  .../ethernet/qualcomm/rmnet/rmnet_handlers.c  | 34 +++++++-----
>  .../net/ethernet/qualcomm/rmnet/rmnet_map.h   |  9 ++--
>  .../qualcomm/rmnet/rmnet_map_command.c        |  9 ++--
>  .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 14 ++---
>  .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   |  2 +-
>  7 files changed, 65 insertions(+), 59 deletions(-)
> 
> --
> 2.55.0.1032.g73a4cd73de-goog

No concerns from our side. For the series - 

Reviewed-by: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>

Re: [PATCH net-next 0/3] net: rmnet: lockless rmnet_fill_info()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 16 Sep 2026 13:22:28 +0000 you wrote:
> rmnet_fill_info() currently relies on RTNL being held, because it
> uses rmnet_get_port_rtnl() to reach the rmnet_port attached to the
> underlying real device.
> 
> While auditing the fields exposed there, it appears that both
> priv->mux_id and port->data_format are written under RTNL but read
> from the data path without any lock. Note that rmnet_newlink() can
> update port->data_format of an already active port, and that
> rmnet_changelink() can change both fields while traffic is flowing.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] net: rmnet: annotate data-races around port->data_format
    https://git.kernel.org/netdev/net-next/c/c863bf6a7d7d
  - [net-next,2/3] net: rmnet: annotate data-races around mux_id
    https://git.kernel.org/netdev/net-next/c/0067187dbe15
  - [net-next,3/3] net: rmnet: no longer rely on RTNL in rmnet_fill_info()
    https://git.kernel.org/netdev/net-next/c/d7c9ba103b06

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html