forked from mirrors/linux
		
	eth: bnxt: always recalculate features after XDP clearing, fix null-deref
Recalculate features when XDP is detached. Before: # ip li set dev eth0 xdp obj xdp_dummy.bpf.o sec xdp # ip li set dev eth0 xdp off # ethtool -k eth0 | grep gro rx-gro-hw: off [requested on] After: # ip li set dev eth0 xdp obj xdp_dummy.bpf.o sec xdp # ip li set dev eth0 xdp off # ethtool -k eth0 | grep gro rx-gro-hw: on The fact that HW-GRO doesn't get re-enabled automatically is just a minor annoyance. The real issue is that the features will randomly come back during another reconfiguration which just happens to invoke netdev_update_features(). The driver doesn't handle reconfiguring two things at a time very robustly. Starting with commit98ba1d931f("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") we only reconfigure the RSS hash table if the "effective" number of Rx rings has changed. If HW-GRO is enabled "effective" number of rings is 2x what user sees. So if we are in the bad state, with HW-GRO re-enablement "pending" after XDP off, and we lower the rings by / 2 - the HW-GRO rings doing 2x and the ethtool -L doing / 2 may cancel each other out, and the: if (old_rx_rings != bp->hw_resc.resv_rx_rings && condition in __bnxt_reserve_rings() will be false. The RSS map won't get updated, and we'll crash with: BUG: kernel NULL pointer dereference, address: 0000000000000168 RIP: 0010:__bnxt_hwrm_vnic_set_rss+0x13a/0x1a0 bnxt_hwrm_vnic_rss_cfg_p5+0x47/0x180 __bnxt_setup_vnic_p5+0x58/0x110 bnxt_init_nic+0xb72/0xf50 __bnxt_open_nic+0x40d/0xab0 bnxt_open_nic+0x2b/0x60 ethtool_set_channels+0x18c/0x1d0 As we try to access a freed ring. The issue is present since XDP support was added, really, but prior to commit98ba1d931f("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") it wasn't causing major issues. Fixes:1054aee823("bnxt_en: Use NETIF_F_GRO_HW.") Fixes:98ba1d931f("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") Reviewed-by: Michael Chan <michael.chan@broadcom.com> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> Link: https://patch.msgid.link/20250109043057.2888953-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
		
							parent
							
								
									b3af60928a
								
							
						
					
					
						commit
						f0aa6a37a3
					
				
					 3 changed files with 21 additions and 13 deletions
				
			
		|  | @ -4708,7 +4708,7 @@ void bnxt_set_ring_params(struct bnxt *bp) | ||||||
| /* Changing allocation mode of RX rings.
 | /* Changing allocation mode of RX rings.
 | ||||||
|  * TODO: Update when extending xdp_rxq_info to support allocation modes. |  * TODO: Update when extending xdp_rxq_info to support allocation modes. | ||||||
|  */ |  */ | ||||||
| int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode) | static void __bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode) | ||||||
| { | { | ||||||
| 	struct net_device *dev = bp->dev; | 	struct net_device *dev = bp->dev; | ||||||
| 
 | 
 | ||||||
|  | @ -4729,15 +4729,30 @@ int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode) | ||||||
| 			bp->rx_skb_func = bnxt_rx_page_skb; | 			bp->rx_skb_func = bnxt_rx_page_skb; | ||||||
| 		} | 		} | ||||||
| 		bp->rx_dir = DMA_BIDIRECTIONAL; | 		bp->rx_dir = DMA_BIDIRECTIONAL; | ||||||
| 		/* Disable LRO or GRO_HW */ |  | ||||||
| 		netdev_update_features(dev); |  | ||||||
| 	} else { | 	} else { | ||||||
| 		dev->max_mtu = bp->max_mtu; | 		dev->max_mtu = bp->max_mtu; | ||||||
| 		bp->flags &= ~BNXT_FLAG_RX_PAGE_MODE; | 		bp->flags &= ~BNXT_FLAG_RX_PAGE_MODE; | ||||||
| 		bp->rx_dir = DMA_FROM_DEVICE; | 		bp->rx_dir = DMA_FROM_DEVICE; | ||||||
| 		bp->rx_skb_func = bnxt_rx_skb; | 		bp->rx_skb_func = bnxt_rx_skb; | ||||||
| 	} | 	} | ||||||
| 	return 0; | } | ||||||
|  | 
 | ||||||
|  | void bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode) | ||||||
|  | { | ||||||
|  | 	__bnxt_set_rx_skb_mode(bp, page_mode); | ||||||
|  | 
 | ||||||
|  | 	if (!page_mode) { | ||||||
|  | 		int rx, tx; | ||||||
|  | 
 | ||||||
|  | 		bnxt_get_max_rings(bp, &rx, &tx, true); | ||||||
|  | 		if (rx > 1) { | ||||||
|  | 			bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS; | ||||||
|  | 			bp->dev->hw_features |= NETIF_F_LRO; | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	/* Update LRO and GRO_HW availability */ | ||||||
|  | 	netdev_update_features(bp->dev); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| static void bnxt_free_vnic_attributes(struct bnxt *bp) | static void bnxt_free_vnic_attributes(struct bnxt *bp) | ||||||
|  | @ -16214,7 +16229,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) | ||||||
| 	if (bp->max_fltr < BNXT_MAX_FLTR) | 	if (bp->max_fltr < BNXT_MAX_FLTR) | ||||||
| 		bp->max_fltr = BNXT_MAX_FLTR; | 		bp->max_fltr = BNXT_MAX_FLTR; | ||||||
| 	bnxt_init_l2_fltr_tbl(bp); | 	bnxt_init_l2_fltr_tbl(bp); | ||||||
| 	bnxt_set_rx_skb_mode(bp, false); | 	__bnxt_set_rx_skb_mode(bp, false); | ||||||
| 	bnxt_set_tpa_flags(bp); | 	bnxt_set_tpa_flags(bp); | ||||||
| 	bnxt_set_ring_params(bp); | 	bnxt_set_ring_params(bp); | ||||||
| 	bnxt_rdma_aux_device_init(bp); | 	bnxt_rdma_aux_device_init(bp); | ||||||
|  |  | ||||||
|  | @ -2846,7 +2846,7 @@ u32 bnxt_fw_health_readl(struct bnxt *bp, int reg_idx); | ||||||
| bool bnxt_bs_trace_avail(struct bnxt *bp, u16 type); | bool bnxt_bs_trace_avail(struct bnxt *bp, u16 type); | ||||||
| void bnxt_set_tpa_flags(struct bnxt *bp); | void bnxt_set_tpa_flags(struct bnxt *bp); | ||||||
| void bnxt_set_ring_params(struct bnxt *); | void bnxt_set_ring_params(struct bnxt *); | ||||||
| int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode); | void bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode); | ||||||
| void bnxt_insert_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr); | void bnxt_insert_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr); | ||||||
| void bnxt_del_one_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr); | void bnxt_del_one_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr); | ||||||
| int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp, unsigned long *bmap, | int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp, unsigned long *bmap, | ||||||
|  |  | ||||||
|  | @ -422,15 +422,8 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog) | ||||||
| 		bnxt_set_rx_skb_mode(bp, true); | 		bnxt_set_rx_skb_mode(bp, true); | ||||||
| 		xdp_features_set_redirect_target(dev, true); | 		xdp_features_set_redirect_target(dev, true); | ||||||
| 	} else { | 	} else { | ||||||
| 		int rx, tx; |  | ||||||
| 
 |  | ||||||
| 		xdp_features_clear_redirect_target(dev); | 		xdp_features_clear_redirect_target(dev); | ||||||
| 		bnxt_set_rx_skb_mode(bp, false); | 		bnxt_set_rx_skb_mode(bp, false); | ||||||
| 		bnxt_get_max_rings(bp, &rx, &tx, true); |  | ||||||
| 		if (rx > 1) { |  | ||||||
| 			bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS; |  | ||||||
| 			bp->dev->hw_features |= NETIF_F_LRO; |  | ||||||
| 		} |  | ||||||
| 	} | 	} | ||||||
| 	bp->tx_nr_rings_xdp = tx_xdp; | 	bp->tx_nr_rings_xdp = tx_xdp; | ||||||
| 	bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tc + tx_xdp; | 	bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tc + tx_xdp; | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Jakub Kicinski
						Jakub Kicinski