forked from mirrors/linux
		
	xdp: double protect netdev->xdp_flags with netdev->lock
Protect xdp_features with netdev->lock. This way pure readers no longer have to take rtnl_lock to access the field. This includes calling NETDEV_XDP_FEAT_CHANGE under the lock. Looks like that's fine for bonding, the only "real" listener, it's the same as ethtool feature change. In terms of normal drivers - only GVE need special consideration (other drivers don't use instance lock or don't support XDP). It calls xdp_set_features_flag() helper from gve_init_priv() which in turn is called from gve_reset_recovery() (locked), or prior to netdev registration. So switch to _locked. Reviewed-by: Joe Damato <jdamato@fastly.com> Acked-by: Stanislav Fomichev <sdf@fomichev.me> Acked-by: Harshitha Ramamurthy <hramamurthy@google.com> Acked-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20250408195956.412733-6-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
		
							parent
							
								
									d02e3b3882
								
							
						
					
					
						commit
						03df156dd3
					
				
					 6 changed files with 16 additions and 4 deletions
				
			
		|  | @ -354,6 +354,7 @@ For devices with locked ops, currently only the following notifiers are | ||||||
| running under the lock: | running under the lock: | ||||||
| * ``NETDEV_REGISTER`` | * ``NETDEV_REGISTER`` | ||||||
| * ``NETDEV_UP`` | * ``NETDEV_UP`` | ||||||
|  | * ``NETDEV_XDP_FEAT_CHANGE`` | ||||||
| 
 | 
 | ||||||
| The following notifiers are running without the lock: | The following notifiers are running without the lock: | ||||||
| * ``NETDEV_UNREGISTER`` | * ``NETDEV_UNREGISTER`` | ||||||
|  |  | ||||||
|  | @ -2185,7 +2185,7 @@ static void gve_set_netdev_xdp_features(struct gve_priv *priv) | ||||||
| 		xdp_features = 0; | 		xdp_features = 0; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	xdp_set_features_flag(priv->dev, xdp_features); | 	xdp_set_features_flag_locked(priv->dev, xdp_features); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device) | static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device) | ||||||
|  |  | ||||||
|  | @ -2526,7 +2526,7 @@ struct net_device { | ||||||
| 	 *	@net_shaper_hierarchy, @reg_state, @threaded | 	 *	@net_shaper_hierarchy, @reg_state, @threaded | ||||||
| 	 * | 	 * | ||||||
| 	 * Double protects: | 	 * Double protects: | ||||||
| 	 *	@up, @moving_ns, @nd_net | 	 *	@up, @moving_ns, @nd_net, @xdp_flags | ||||||
| 	 * | 	 * | ||||||
| 	 * Double ops protects: | 	 * Double ops protects: | ||||||
| 	 *	@real_num_rx_queues, @real_num_tx_queues | 	 *	@real_num_rx_queues, @real_num_tx_queues | ||||||
|  |  | ||||||
|  | @ -616,6 +616,7 @@ struct xdp_metadata_ops { | ||||||
| u32 bpf_xdp_metadata_kfunc_id(int id); | u32 bpf_xdp_metadata_kfunc_id(int id); | ||||||
| bool bpf_dev_bound_kfunc_id(u32 btf_id); | bool bpf_dev_bound_kfunc_id(u32 btf_id); | ||||||
| void xdp_set_features_flag(struct net_device *dev, xdp_features_t val); | void xdp_set_features_flag(struct net_device *dev, xdp_features_t val); | ||||||
|  | void xdp_set_features_flag_locked(struct net_device *dev, xdp_features_t val); | ||||||
| void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg); | void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg); | ||||||
| void xdp_features_clear_redirect_target(struct net_device *dev); | void xdp_features_clear_redirect_target(struct net_device *dev); | ||||||
| #else | #else | ||||||
|  |  | ||||||
|  | @ -20,6 +20,7 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event, | ||||||
| 	switch (cmd) { | 	switch (cmd) { | ||||||
| 	case NETDEV_REGISTER: | 	case NETDEV_REGISTER: | ||||||
| 	case NETDEV_UP: | 	case NETDEV_UP: | ||||||
|  | 	case NETDEV_XDP_FEAT_CHANGE: | ||||||
| 		netdev_ops_assert_locked(dev); | 		netdev_ops_assert_locked(dev); | ||||||
| 		fallthrough; | 		fallthrough; | ||||||
| 	case NETDEV_DOWN: | 	case NETDEV_DOWN: | ||||||
|  | @ -58,7 +59,6 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event, | ||||||
| 	case NETDEV_OFFLOAD_XSTATS_DISABLE: | 	case NETDEV_OFFLOAD_XSTATS_DISABLE: | ||||||
| 	case NETDEV_OFFLOAD_XSTATS_REPORT_USED: | 	case NETDEV_OFFLOAD_XSTATS_REPORT_USED: | ||||||
| 	case NETDEV_OFFLOAD_XSTATS_REPORT_DELTA: | 	case NETDEV_OFFLOAD_XSTATS_REPORT_DELTA: | ||||||
| 	case NETDEV_XDP_FEAT_CHANGE: |  | ||||||
| 		ASSERT_RTNL(); | 		ASSERT_RTNL(); | ||||||
| 		break; | 		break; | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -17,6 +17,7 @@ | ||||||
| #include <net/page_pool/helpers.h> | #include <net/page_pool/helpers.h> | ||||||
| 
 | 
 | ||||||
| #include <net/hotdata.h> | #include <net/hotdata.h> | ||||||
|  | #include <net/netdev_lock.h> | ||||||
| #include <net/xdp.h> | #include <net/xdp.h> | ||||||
| #include <net/xdp_priv.h> /* struct xdp_mem_allocator */ | #include <net/xdp_priv.h> /* struct xdp_mem_allocator */ | ||||||
| #include <trace/events/xdp.h> | #include <trace/events/xdp.h> | ||||||
|  | @ -991,17 +992,26 @@ static int __init xdp_metadata_init(void) | ||||||
| } | } | ||||||
| late_initcall(xdp_metadata_init); | late_initcall(xdp_metadata_init); | ||||||
| 
 | 
 | ||||||
| void xdp_set_features_flag(struct net_device *dev, xdp_features_t val) | void xdp_set_features_flag_locked(struct net_device *dev, xdp_features_t val) | ||||||
| { | { | ||||||
| 	val &= NETDEV_XDP_ACT_MASK; | 	val &= NETDEV_XDP_ACT_MASK; | ||||||
| 	if (dev->xdp_features == val) | 	if (dev->xdp_features == val) | ||||||
| 		return; | 		return; | ||||||
| 
 | 
 | ||||||
|  | 	netdev_assert_locked_or_invisible(dev); | ||||||
| 	dev->xdp_features = val; | 	dev->xdp_features = val; | ||||||
| 
 | 
 | ||||||
| 	if (dev->reg_state == NETREG_REGISTERED) | 	if (dev->reg_state == NETREG_REGISTERED) | ||||||
| 		call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev); | 		call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev); | ||||||
| } | } | ||||||
|  | EXPORT_SYMBOL_GPL(xdp_set_features_flag_locked); | ||||||
|  | 
 | ||||||
|  | void xdp_set_features_flag(struct net_device *dev, xdp_features_t val) | ||||||
|  | { | ||||||
|  | 	netdev_lock(dev); | ||||||
|  | 	xdp_set_features_flag_locked(dev, val); | ||||||
|  | 	netdev_unlock(dev); | ||||||
|  | } | ||||||
| EXPORT_SYMBOL_GPL(xdp_set_features_flag); | EXPORT_SYMBOL_GPL(xdp_set_features_flag); | ||||||
| 
 | 
 | ||||||
| void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg) | void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Jakub Kicinski
						Jakub Kicinski