1
0
Fork 0
mirror of synced 2025-03-06 20:59:54 +01:00

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 commit 98ba1d931f ("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 commit 98ba1d931f ("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:
Jakub Kicinski 2025-01-08 20:30:57 -08:00
parent b3af60928a
commit f0aa6a37a3
3 changed files with 21 additions and 13 deletions

View file

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

View file

@ -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,

View file

@ -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;