Merge branch 'net-rmnet-lockless-rmnet_fill_info'

Eric Dumazet says:

====================
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().
====================

Link: https://patch.msgid.link/20260916132231.1780091-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

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;
 
@@ -337,25 +337,27 @@ 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);
 		}
 	}
 
 	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;
 		}
@@ -375,32 +377,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 = {
@@ -417,12 +411,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)
@@ -431,7 +429,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;
 	}
 

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,

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;
 
@@ -255,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)

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_ */

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);
 }

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;
 

drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c

@@ -341,7 +341,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");
 	}