bpf: do not WARN in bpf_warn_invalid_xdp_action()
Subsystems (30)
Paolo Abeni
maintainercover bpf: do not WARN in bpf_warn_invalid_xdp_action()
The mentioned WARN is quite noisy, especially vs fuzzers and apparently used only to track the relevant BPF program and/or involved driver.
The first patch replace it with a pr_warn_once(), and the 2nd patch allow dumps relevant info to track the reported issue.
This is quite invasive, but the mentioned WARN makes the hunt for some bugs reported by syzkaller quite difficult.
Paolo Abeni (2):
bpf: do not WARN in bpf_warn_invalid_xdp_action()
bpf: let bpf_warn_invalid_xdp_action() report more info
drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 +- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 +- drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/igc/igc_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 +- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +- drivers/net/ethernet/marvell/mvneta.c | 2 +- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +- drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 2 +- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 +- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_fp.c | 2 +- drivers/net/ethernet/sfc/rx.c | 2 +- drivers/net/ethernet/socionext/netsec.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/ti/cpsw_priv.c | 2 +- drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/tun.c | 2 +- drivers/net/veth.c | 4 ++-- drivers/net/virtio_net.c | 4 ++-- drivers/net/xen-netfront.c | 2 +- include/linux/filter.h | 2 +- kernel/bpf/cpumap.c | 4 ++-- kernel/bpf/devmap.c | 4 ++-- net/core/dev.c | 2 +- net/core/filter.c | 8 ++++---- 36 files changed, 43 insertions(+), 43 deletions(-)
Signature · 1 line
2.33.1
Paolo Abeni
maintainerRFC PATCH 1/2 bpf: do not WARN in bpf_warn_invalid_xdp_action()
The WARN_ONCE() in bpf_warn_invalid_xdp_action() can be triggered by any bugged program, and even attaching a correct program to a NIC not supporting the given action.
The resulting splat, beyond polluting the logs, fouls automated tools:
e.g. a syzkaller reproducers using an XDP program returning an unsupported action will never pass validation.
Replace the WARN_ONCE with a less intrusive pr_warn_once().
Signed-off-by Paolo Abeni <[email protected]>
--- net/core/filter.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
net/core/filter.c +3 -3
@@ -8183,9 +8183,9 @@ void bpf_warn_invalid_xdp_action(u32 act) {const u32 act_max =XDP_REDIRECT ; -WARN_ONCE (1 ,"%s XDP return value %u, expect packet loss!\n" , - act > act_max ?"Illegal" :"Driver unsupported" , - act); +pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , + act > act_max ?"Illegal" :"Driver unsupported" , + act); }EXPORT_SYMBOL_GPL (bpf_warn_invalid_xdp_action);
Signature · 1 line
2.33.1
Quoted 11 lines · Paolo Abeni
The
WARN_ONCE()inbpf_warn_invalid_xdp_action()can be triggered by any bugged program, and even attaching a correct program to a NIC not supporting the given action.The resulting splat, beyond polluting the logs, fouls automated tools:
e.g. a syzkaller reproducers using an XDP program returning an unsupported action will never pass validation.Replace the
WARN_ONCEwith a less intrusivepr_warn_once().Signed-off-by Paolo Abeni <[email protected]>
Paolo Abeni
maintainerRFC PATCH 2/2 bpf: let bpf_warn_invalid_xdp_action() report more info
In reply to Paolo AbeniIn non trivial scenarios, the single action id is not sufficient to identify the program causing the warning. Let's additionally include the program id, the attach type and the relevant device name.
Signed-off-by Paolo Abeni <[email protected]>
--- drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 +- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 +- drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/igc/igc_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 +- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +- drivers/net/ethernet/marvell/mvneta.c | 2 +- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +- drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 2 +- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 +- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_fp.c | 2 +- drivers/net/ethernet/sfc/rx.c | 2 +- drivers/net/ethernet/socionext/netsec.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/ti/cpsw_priv.c | 2 +- drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/tun.c | 2 +- drivers/net/veth.c | 4 ++-- drivers/net/virtio_net.c | 4 ++-- drivers/net/xen-netfront.c | 2 +- include/linux/filter.h | 2 +- kernel/bpf/cpumap.c | 4 ++-- kernel/bpf/devmap.c | 4 ++-- net/core/dev.c | 2 +- net/core/filter.c | 6 +++--- 36 files changed, 42 insertions(+), 42 deletions(-)
drivers/net/ethernet/amazon/ena/ena_netdev.c +1 -1
@@ -434,7 +434,7 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) xdp_stat = &rx_ring->rx_stats.xdp_pass;break ;default : -bpf_warn_invalid_xdp_action (verdict); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, verdict); xdp_stat = &rx_ring->rx_stats.xdp_invalid; }
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +1 -1
@@ -195,7 +195,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, *event |=BNXT_REDIRECT_EVENT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (bp->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (bp->dev, xdp_prog, act);
drivers/net/ethernet/cavium/thunder/nicvf_main.c +1 -1
@@ -590,7 +590,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,nicvf_xdp_sq_append_pkt (nic, sq, (u64 )xdp.data, dma_addr, len);return true;default : -bpf_warn_invalid_xdp_action (action); +bpf_warn_invalid_xdp_action (nic->netdev, prog, action); fallthrough;case XDP_ABORTED :trace_xdp_exception (nic->netdev, prog, action);
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +1 -1
@@ -2623,7 +2623,7 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, }break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +1 -1
@@ -374,7 +374,7 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv,dpaa2_eth_xdp_enqueue (priv, ch, fd, vaddr, rx_fq->flowid);break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);
drivers/net/ethernet/freescale/enetc/enetc.c +1 -1
@@ -1547,7 +1547,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,switch (xdp_act) {default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (rx_ring->ndev, prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rx_ring->ndev, prog, xdp_act);
drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -1
@@ -2322,7 +2322,7 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) result =I40E_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/i40e/i40e_xsk.c +1 -1
@@ -176,7 +176,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/ice/ice_txrx.c +1 -1
@@ -561,7 +561,7 @@ ice_run_xdp(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;return ICE_XDP_REDIR ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/ice/ice_xsk.c +1 -1
@@ -482,7 +482,7 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/igb/igb_main.c +1 -1
@@ -8422,7 +8422,7 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, result =IGB_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/igc/igc_main.c +1 -1
@@ -2231,7 +2231,7 @@ static int __igc_xdp_run_prog(struct igc_adapter *adapter,return IGC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
@@ -2235,7 +2235,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, result =IXGBE_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +1 -1
@@ -131,7 +131,7 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +1 -1
@@ -1070,7 +1070,7 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:
drivers/net/ethernet/marvell/mvneta.c +1 -1
@@ -2212,7 +2212,7 @@ mvneta_run_xdp(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,mvneta_xdp_put_buff (pp, rxq, xdp, sinfo, sync);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pp->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (pp->dev, prog, act);
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1 -1
@@ -3820,7 +3820,7 @@ mvpp2_run_xdp(struct mvpp2_port *port, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (port->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (port->dev, prog, act);
drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c +1 -1
@@ -1198,7 +1198,7 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,put_page (page);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pfvf->netdev, prog, act);break ;case XDP_ABORTED :trace_xdp_exception (pfvf->netdev, prog, act);
drivers/net/ethernet/mellanox/mlx4/en_rx.c +1 -1
@@ -812,7 +812,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int budtrace_xdp_exception (dev, xdp_prog, act); goto xdp_drop_no_cnt;/* Drop on xmit failure */ default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c +1 -1
@@ -151,7 +151,7 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di, rq->stats->xdp_redirect++;return true;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->netdev, prog, act); fallthrough;case XDP_ABORTED : xdp_abort:
drivers/net/ethernet/netronome/nfp/nfp_net_common.c +1 -1
@@ -1944,7 +1944,7 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) xdp_prog, act);continue ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dp->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dp->netdev, xdp_prog, act);
drivers/net/ethernet/qlogic/qede/qede_fp.c +1 -1
@@ -1152,7 +1152,7 @@ static bool qede_rx_xdp(struct qede_dev *edev,qede_rx_bd_ring_consume (rxq);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (edev->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (edev->ndev, prog, act);
drivers/net/ethernet/sfc/rx.c +1 -1
@@ -338,7 +338,7 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (efx->net_dev, xdp_prog, xdp_act);efx_free_rx_buffers (rx_queue, rx_buf,1 ); channel->n_rx_xdp_bad_drops++;trace_xdp_exception (efx->net_dev, xdp_prog, xdp_act);
drivers/net/ethernet/socionext/netsec.c +1 -1
@@ -933,7 +933,7 @@ static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->ndev, prog, act);
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -1
@@ -4690,7 +4690,7 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, res =STMMAC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->dev, prog, act);
drivers/net/ethernet/ti/cpsw_priv.c +1 -1
@@ -1360,7 +1360,7 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,xdp_do_flush_map ();break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (ndev, prog, act);
drivers/net/hyperv/netvsc_bpf.c +1 -1
@@ -68,7 +68,7 @@ u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); } out:
drivers/net/tun.c +1 -1
@@ -1546,7 +1546,7 @@ static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (tun->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (tun->dev, xdp_prog, act);
drivers/net/veth.c +2 -2
@@ -651,7 +651,7 @@ static struct xdp_frame *veth_xdp_rcv_one(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);
@@ -801,7 +801,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);
drivers/net/virtio_net.c +2 -2
@@ -812,7 +812,7 @@ static struct sk_buff *receive_small(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);
@@ -1025,7 +1025,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);
drivers/net/xen-netfront.c +1 -1
@@ -930,7 +930,7 @@ static u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (queue->info->netdev, prog, act); }return act;
include/linux/filter.h +1 -1
@@ -1030,7 +1030,7 @@ void xdp_do_flush(void);*/ #define xdp_do_flush_map xdp_do_flush -void bpf_warn_invalid_xdp_action (u32 act); +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act);#ifdef CONFIG_INET struct sock *bpf_run_sk_reuseport(struct sock_reuseport *reuse, struct sock *sk,
kernel/bpf/cpumap.c +2 -2
@@ -195,7 +195,7 @@ static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu, }return ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, rcpu->prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (skb->dev, rcpu->prog, act);
@@ -254,7 +254,7 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (xdpf->dev_rx, rcpu->prog, act); fallthrough;case XDP_DROP :xdp_return_frame (xdpf);
kernel/bpf/devmap.c +2 -2
@@ -348,7 +348,7 @@ static int dev_map_bpf_prog_run(struct bpf_prog *xdp_prog, frames[nframes++] = xdpf;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);
@@ -507,7 +507,7 @@ static u32 dev_map_bpf_prog_run_skb(struct sk_buff *skb, struct bpf_dtab_netdev__skb_push (skb, skb->mac_len);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dst->dev, dst->xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dst->dev, dst->xdp_prog, act);
net/core/dev.c +1 -1
@@ -4824,7 +4824,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (skb->dev, xdp_prog, act);
net/core/filter.c +3 -3
@@ -8179,13 +8179,13 @@ static bool xdp_is_valid_access(int off, int size,return __is_valid_xdp_access (off, size); } -void bpf_warn_invalid_xdp_action (u32 act) +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act) {const u32 act_max =XDP_REDIRECT ; -pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type); }EXPORT_SYMBOL_GPL (bpf_warn_invalid_xdp_action);
Signature · 1 line
2.33.1
Re: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 484 lines · Paolo Abeni
In non trivial scenarios, the single action id is not sufficient to identify the program causing the warning. Let's additionally include the program id, the attach type and the relevant device name.
Signed-off-by Paolo Abeni <[email protected]>--- drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 +- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 +- drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/igc/igc_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 +- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +- drivers/net/ethernet/marvell/mvneta.c | 2 +- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +- drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 2 +- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 +- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_fp.c | 2 +- drivers/net/ethernet/sfc/rx.c | 2 +- drivers/net/ethernet/socionext/netsec.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/ti/cpsw_priv.c | 2 +- drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/tun.c | 2 +- drivers/net/veth.c | 4 ++-- drivers/net/virtio_net.c | 4 ++-- drivers/net/xen-netfront.c | 2 +- include/linux/filter.h | 2 +- kernel/bpf/cpumap.c | 4 ++-- kernel/bpf/devmap.c | 4 ++-- net/core/dev.c | 2 +- net/core/filter.c | 6 +++--- 36 files changed, 42 insertions(+), 42 deletions(-)drivers/net/ethernet/amazon/ena/ena_netdev.c +1
-1@@ -434,7 +434,7 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) xdp_stat = &rx_ring->rx_stats.xdp_pass;break ;default : -bpf_warn_invalid_xdp_action (verdict); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, verdict); xdp_stat = &rx_ring->rx_stats.xdp_invalid; }drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +1
-1@@ -195,7 +195,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, *event |=BNXT_REDIRECT_EVENT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (bp->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (bp->dev, xdp_prog, act);drivers/net/ethernet/cavium/thunder/nicvf_main.c +1
-1@@ -590,7 +590,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,nicvf_xdp_sq_append_pkt (nic, sq, (u64 )xdp.data, dma_addr, len);return true;default : -bpf_warn_invalid_xdp_action (action); +bpf_warn_invalid_xdp_action (nic->netdev, prog, action); fallthrough;case XDP_ABORTED :trace_xdp_exception (nic->netdev, prog, action);drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +1
-1@@ -2623,7 +2623,7 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, }break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +1
-1@@ -374,7 +374,7 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv,dpaa2_eth_xdp_enqueue (priv, ch, fd, vaddr, rx_fq->flowid);break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/freescale/enetc/enetc.c +1
-1@@ -1547,7 +1547,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,switch (xdp_act) {default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (rx_ring->ndev, prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rx_ring->ndev, prog, xdp_act);drivers/net/ethernet/intel/i40e/i40e_txrx.c +1
-1@@ -2322,7 +2322,7 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) result =I40E_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/i40e/i40e_xsk.c +1
-1@@ -176,7 +176,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ice/ice_txrx.c +1
-1@@ -561,7 +561,7 @@ ice_run_xdp(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;return ICE_XDP_REDIR ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ice/ice_xsk.c +1
-1@@ -482,7 +482,7 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/igb/igb_main.c +1
-1@@ -8422,7 +8422,7 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, result =IGB_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/igc/igc_main.c +1
-1@@ -2231,7 +2231,7 @@ static int __igc_xdp_run_prog(struct igc_adapter *adapter,return IGC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1
-1@@ -2235,7 +2235,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, result =IXGBE_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +1
-1@@ -131,7 +131,7 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +1
-1@@ -1070,7 +1070,7 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/marvell/mvneta.c +1
-1@@ -2212,7 +2212,7 @@ mvneta_run_xdp(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,mvneta_xdp_put_buff (pp, rxq, xdp, sinfo, sync);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pp->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (pp->dev, prog, act);drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1
-1@@ -3820,7 +3820,7 @@ mvpp2_run_xdp(struct mvpp2_port *port, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (port->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (port->dev, prog, act);drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c +1
-1@@ -1198,7 +1198,7 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,put_page (page);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pfvf->netdev, prog, act);break ;case XDP_ABORTED :trace_xdp_exception (pfvf->netdev, prog, act);drivers/net/ethernet/mellanox/mlx4/en_rx.c +1
-1@@ -812,7 +812,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int budtrace_xdp_exception (dev, xdp_prog, act); goto xdp_drop_no_cnt;/* Drop on xmit failure */ default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c +1
-1@@ -151,7 +151,7 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di, rq->stats->xdp_redirect++;return true;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->netdev, prog, act); fallthrough;case XDP_ABORTED : xdp_abort:drivers/net/ethernet/netronome/nfp/nfp_net_common.c +1
-1@@ -1944,7 +1944,7 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) xdp_prog, act);continue ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dp->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dp->netdev, xdp_prog, act);drivers/net/ethernet/qlogic/qede/qede_fp.c +1
-1@@ -1152,7 +1152,7 @@ static bool qede_rx_xdp(struct qede_dev *edev,qede_rx_bd_ring_consume (rxq);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (edev->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (edev->ndev, prog, act);drivers/net/ethernet/sfc/rx.c +1
-1@@ -338,7 +338,7 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (efx->net_dev, xdp_prog, xdp_act);efx_free_rx_buffers (rx_queue, rx_buf,1 ); channel->n_rx_xdp_bad_drops++;trace_xdp_exception (efx->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/socionext/netsec.c +1
-1@@ -933,7 +933,7 @@ static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->ndev, prog, act);drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-1@@ -4690,7 +4690,7 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, res =STMMAC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->dev, prog, act);drivers/net/ethernet/ti/cpsw_priv.c +1
-1@@ -1360,7 +1360,7 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,xdp_do_flush_map ();break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (ndev, prog, act);drivers/net/hyperv/netvsc_bpf.c +1
-1@@ -68,7 +68,7 @@ u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); } out:drivers/net/tun.c +1
-1@@ -1546,7 +1546,7 @@ static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (tun->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (tun->dev, xdp_prog, act);drivers/net/veth.c +2
-2@@ -651,7 +651,7 @@ static struct xdp_frame *veth_xdp_rcv_one(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);@@ -801,7 +801,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);drivers/net/virtio_net.c +2
-2@@ -812,7 +812,7 @@ static struct sk_buff *receive_small(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);@@ -1025,7 +1025,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);drivers/net/xen-netfront.c +1
-1@@ -930,7 +930,7 @@ static u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (queue->info->netdev, prog, act); }return act;include/linux/filter.h +1
-1@@ -1030,7 +1030,7 @@ void xdp_do_flush(void);*/ #define xdp_do_flush_map xdp_do_flush -void bpf_warn_invalid_xdp_action (u32 act); +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act);#ifdef CONFIG_INET struct sock *bpf_run_sk_reuseport(struct sock_reuseport *reuse, struct sock *sk,kernel/bpf/cpumap.c +0
-0
kernel/bpf/cpumap.c static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu, · line 199
Quoted 6 lines
kernel/bpf/cpumap.c +1
-1@@ -195,7 +195,7 @@ static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu, }return ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, rcpu->prog, act);
I'm not sure if including the device from the map run points is the right thing to do: the error message refers to "driver unsupported" actions which is not the case for these instances. So I think it would make more sense to not include the device for these, and adjust the error message correspondingly.
Quoted 55 lines
kernel/bpf/cpumap.c +0
-0fallthrough;case XDP_ABORTED :kernel/bpf/cpumap.c +1
-1trace_xdp_exception (skb->dev, rcpu->prog, act);@@ -254,7 +254,7 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (xdpf->dev_rx, rcpu->prog, act); fallthrough;case XDP_DROP :xdp_return_frame (xdpf);kernel/bpf/devmap.c +2
-2@@ -348,7 +348,7 @@ static int dev_map_bpf_prog_run(struct bpf_prog *xdp_prog, frames[nframes++] = xdpf;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);@@ -507,7 +507,7 @@ static u32 dev_map_bpf_prog_run_skb(struct sk_buff *skb, struct bpf_dtab_netdev__skb_push (skb, skb->mac_len);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dst->dev, dst->xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dst->dev, dst->xdp_prog, act);net/core/dev.c +1
-1@@ -4824,7 +4824,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (skb->dev, xdp_prog, act);net/core/filter.c +0
-0@@ -8179,13 +8179,13 @@ static bool xdp_is_valid_access(int off, int size,return __is_valid_xdp_access (off, size); }
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 10 lines
net/core/filter.c +3
-3-void bpf_warn_invalid_xdp_action (u32 act) +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act) {const u32 act_max =XDP_REDIRECT ; -pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);
This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
-Toke
Paolo Abeni
maintainerRe: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 559 lines · Toke Høiland-Jørgensen
Quoted 484 lines · Paolo Abeni
In non trivial scenarios, the single action id is not sufficient to identify the program causing the warning. Let's additionally include the program id, the attach type and the relevant device name.
Signed-off-by Paolo Abeni <[email protected]>--- drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 +- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 +- drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/igc/igc_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 +- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +- drivers/net/ethernet/marvell/mvneta.c | 2 +- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +- drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 2 +- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 +- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_fp.c | 2 +- drivers/net/ethernet/sfc/rx.c | 2 +- drivers/net/ethernet/socionext/netsec.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/ti/cpsw_priv.c | 2 +- drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/tun.c | 2 +- drivers/net/veth.c | 4 ++-- drivers/net/virtio_net.c | 4 ++-- drivers/net/xen-netfront.c | 2 +- include/linux/filter.h | 2 +- kernel/bpf/cpumap.c | 4 ++-- kernel/bpf/devmap.c | 4 ++-- net/core/dev.c | 2 +- net/core/filter.c | 6 +++--- 36 files changed, 42 insertions(+), 42 deletions(-)drivers/net/ethernet/amazon/ena/ena_netdev.c +1
-1@@ -434,7 +434,7 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) xdp_stat = &rx_ring->rx_stats.xdp_pass;break ;default : -bpf_warn_invalid_xdp_action (verdict); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, verdict); xdp_stat = &rx_ring->rx_stats.xdp_invalid; }drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +1
-1@@ -195,7 +195,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, *event |=BNXT_REDIRECT_EVENT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (bp->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (bp->dev, xdp_prog, act);drivers/net/ethernet/cavium/thunder/nicvf_main.c +1
-1@@ -590,7 +590,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,nicvf_xdp_sq_append_pkt (nic, sq, (u64 )xdp.data, dma_addr, len);return true;default : -bpf_warn_invalid_xdp_action (action); +bpf_warn_invalid_xdp_action (nic->netdev, prog, action); fallthrough;case XDP_ABORTED :trace_xdp_exception (nic->netdev, prog, action);drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +1
-1@@ -2623,7 +2623,7 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, }break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +1
-1@@ -374,7 +374,7 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv,dpaa2_eth_xdp_enqueue (priv, ch, fd, vaddr, rx_fq->flowid);break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (priv->net_dev, xdp_prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/freescale/enetc/enetc.c +1
-1@@ -1547,7 +1547,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,switch (xdp_act) {default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (rx_ring->ndev, prog, xdp_act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rx_ring->ndev, prog, xdp_act);drivers/net/ethernet/intel/i40e/i40e_txrx.c +1
-1@@ -2322,7 +2322,7 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) result =I40E_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/i40e/i40e_xsk.c +1
-1@@ -176,7 +176,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ice/ice_txrx.c +1
-1@@ -561,7 +561,7 @@ ice_run_xdp(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;return ICE_XDP_REDIR ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ice/ice_xsk.c +1
-1@@ -482,7 +482,7 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/igb/igb_main.c +1
-1@@ -8422,7 +8422,7 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, result =IGB_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/igc/igc_main.c +1
-1@@ -2231,7 +2231,7 @@ static int __igc_xdp_run_prog(struct igc_adapter *adapter,return IGC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (adapter->netdev, prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1
-1@@ -2235,7 +2235,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, result =IXGBE_XDP_REDIR ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +1
-1@@ -131,7 +131,7 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +1
-1@@ -1070,7 +1070,7 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, goto out_failure;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rx_ring->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED : out_failure:drivers/net/ethernet/marvell/mvneta.c +1
-1@@ -2212,7 +2212,7 @@ mvneta_run_xdp(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,mvneta_xdp_put_buff (pp, rxq, xdp, sinfo, sync);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pp->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (pp->dev, prog, act);drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +1
-1@@ -3820,7 +3820,7 @@ mvpp2_run_xdp(struct mvpp2_port *port, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (port->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (port->dev, prog, act);drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c +1
-1@@ -1198,7 +1198,7 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,put_page (page);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (pfvf->netdev, prog, act);break ;case XDP_ABORTED :trace_xdp_exception (pfvf->netdev, prog, act);drivers/net/ethernet/mellanox/mlx4/en_rx.c +1
-1@@ -812,7 +812,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int budtrace_xdp_exception (dev, xdp_prog, act); goto xdp_drop_no_cnt;/* Drop on xmit failure */ default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c +1
-1@@ -151,7 +151,7 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di, rq->stats->xdp_redirect++;return true;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->netdev, prog, act); fallthrough;case XDP_ABORTED : xdp_abort:drivers/net/ethernet/netronome/nfp/nfp_net_common.c +1
-1@@ -1944,7 +1944,7 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) xdp_prog, act);continue ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dp->netdev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dp->netdev, xdp_prog, act);drivers/net/ethernet/qlogic/qede/qede_fp.c +1
-1@@ -1152,7 +1152,7 @@ static bool qede_rx_xdp(struct qede_dev *edev,qede_rx_bd_ring_consume (rxq);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (edev->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (edev->ndev, prog, act);drivers/net/ethernet/sfc/rx.c +1
-1@@ -338,7 +338,7 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,break ;default : -bpf_warn_invalid_xdp_action (xdp_act); +bpf_warn_invalid_xdp_action (efx->net_dev, xdp_prog, xdp_act);efx_free_rx_buffers (rx_queue, rx_buf,1 ); channel->n_rx_xdp_bad_drops++;trace_xdp_exception (efx->net_dev, xdp_prog, xdp_act);drivers/net/ethernet/socionext/netsec.c +1
-1@@ -933,7 +933,7 @@ static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->ndev, prog, act);drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-1@@ -4690,7 +4690,7 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, res =STMMAC_XDP_REDIRECT ;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (priv->dev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (priv->dev, prog, act);drivers/net/ethernet/ti/cpsw_priv.c +1
-1@@ -1360,7 +1360,7 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp,xdp_do_flush_map ();break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (ndev, prog, act);drivers/net/hyperv/netvsc_bpf.c +1
-1@@ -68,7 +68,7 @@ u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (ndev, prog, act); } out:drivers/net/tun.c +1
-1@@ -1546,7 +1546,7 @@ static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (tun->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (tun->dev, xdp_prog, act);drivers/net/veth.c +2
-2@@ -651,7 +651,7 @@ static struct xdp_frame *veth_xdp_rcv_one(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);@@ -801,7 +801,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (rq->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (rq->dev, xdp_prog, act);drivers/net/virtio_net.c +2
-2@@ -812,7 +812,7 @@ static struct sk_buff *receive_small(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);@@ -1025,7 +1025,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,rcu_read_unlock (); goto xdp_xmit;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (vi->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (vi->dev, xdp_prog, act);drivers/net/xen-netfront.c +1
-1@@ -930,7 +930,7 @@ static u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (queue->info->netdev, prog, act); }return act;include/linux/filter.h +1
-1@@ -1030,7 +1030,7 @@ void xdp_do_flush(void);*/ #define xdp_do_flush_map xdp_do_flush -void bpf_warn_invalid_xdp_action (u32 act); +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act);#ifdef CONFIG_INET struct sock *bpf_run_sk_reuseport(struct sock_reuseport *reuse, struct sock *sk,kernel/bpf/cpumap.c +0
-0kernel/bpf/cpumap.c static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu, · line 199
Quoted 6 lines
kernel/bpf/cpumap.c +1
-1@@ -195,7 +195,7 @@ static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu, }return ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, rcpu->prog, act);I'm not sure if including the device from the map run points is the right thing to do: the error message refers to "driver unsupported" actions which is not the case for these instances. So I think it would make more sense to not include the device for these, and adjust the error message correspondingly.
Quoted 60 lines
kernel/bpf/cpumap.c +0
-0fallthrough;case XDP_ABORTED :kernel/bpf/cpumap.c +1
-1trace_xdp_exception (skb->dev, rcpu->prog, act);@@ -254,7 +254,7 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, }break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (xdpf->dev_rx, rcpu->prog, act); fallthrough;case XDP_DROP :xdp_return_frame (xdpf);kernel/bpf/devmap.c +2
-2@@ -348,7 +348,7 @@ static int dev_map_bpf_prog_run(struct bpf_prog *xdp_prog, frames[nframes++] = xdpf;break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dev, xdp_prog, act);@@ -507,7 +507,7 @@ static u32 dev_map_bpf_prog_run_skb(struct sk_buff *skb, struct bpf_dtab_netdev__skb_push (skb, skb->mac_len);break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (dst->dev, dst->xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (dst->dev, dst->xdp_prog, act);net/core/dev.c +1
-1@@ -4824,7 +4824,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,case XDP_PASS :break ;default : -bpf_warn_invalid_xdp_action (act); +bpf_warn_invalid_xdp_action (skb->dev, xdp_prog, act); fallthrough;case XDP_ABORTED :trace_xdp_exception (skb->dev, xdp_prog, act);net/core/filter.c +1
-1@@ -8179,13 +8179,13 @@ static bool xdp_is_valid_access(int off, int size,return __is_valid_xdp_access (off, size); } -void bpf_warn_invalid_xdp_action (u32 act) +void bpf_warn_invalid_xdp_action (struct net_device *dev,struct bpf_prog *prog,u32 act) {const u32 act_max =XDP_REDIRECT ;
Quoted 10 lines
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 5 lines
net/core/filter.c +2
-2-pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
I think this patch is needed because the WARN_ONCE splat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.
Cheers,
Paolo
Re: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 11 lines · Paolo Abeni
Quoted 10 lines
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 5 lines
net/core/filter.c +2
-2-pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Quoted 6 lines
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
I think this patch is needed because the
WARN_ONCEsplat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.
Ah, right, good point. Pointing that out in the commit message might be a good idea; otherwise people may miss that ;)
-Toke
Re: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 16 lines · Toke Høiland-Jørgensen
Quoted 14 lines · Paolo Abeni
Quoted 10 lines
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 5 lines
net/core/filter.c +2
-2-pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
Quoted 6 lines
Quoted 3 lines
I think this patch is needed because the
WARN_ONCEsplat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.Ah, right, good point. Pointing that out in the commit message might be a good idea; otherwise people may miss that ;)
Though it's quite a churn across the drivers I think extra verbosity here is justified. I'd only suggest to print stable things. Like prog->aux->id probably has little value for the person looking at the logs. That prog id is likely gone. If it was prog->aux->name it would be more helpful.
Same with expected_attach_type. Why print it at all?
tracepoint is probably good idea too.
Paolo Abeni
maintainerRe: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Hello,
Quoted 24 lines · Alexei Starovoitov
Quoted 16 lines · Toke Høiland-Jørgensen
Quoted 14 lines · Paolo Abeni
Quoted 10 lines
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 5 lines
net/core/filter.c +2
-2-pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
Quoted 6 lines
Quoted 3 lines
I think this patch is needed because the
WARN_ONCEsplat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.Ah, right, good point. Pointing that out in the commit message might be a good idea; otherwise people may miss that ;)
Quoted 6 lines
Though it's quite a churn across the drivers I think extra verbosity here is justified. I'd only suggest to print stable things. Like
prog->aux->idprobably has little value for the person looking at the logs. That prog id is likely gone. If it wasprog->aux->nameit would be more helpful.
Same withexpected_attach_type. Why print it at all?
tracepoint is probably good idea too.
Thanks for the feedback.
I tried to select the additional arguments to allow the user/admin tracking down which program is causing the issue and why. I'm a complete newbe wrt XDP, so likely my choice were naive.
I thought the id identifies the program in an unambiguous manner. I understand the program could be unloaded meanwhile, but if that is not the case the id should be quite useful. Perhaps we could dump both the id and the name?
I included the attach type as different types support/allow different
actions: the same program could cause the warning or not depending on it. If that is not useful I can drop the attach type from the next iteration.
Thanks!
Paolo
Re: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 45 lines · Paolo Abeni
Hello,
Quoted 24 lines · Alexei Starovoitov
Quoted 16 lines · Toke Høiland-Jørgensen
Quoted 14 lines · Paolo Abeni
Quoted 10 lines
net/core/filter.c static bool xdp_is_valid_access(int off, int size, · line 8188
Quoted 5 lines
net/core/filter.c +2
-2-pr_warn_once ("%s XDP return value %u, expect packet loss!\n" , +pr_warn_once ("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n" , act > act_max ?"Illegal" :"Driver unsupported" , - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
Quoted 6 lines
Quoted 3 lines
I think this patch is needed because the
WARN_ONCEsplat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.Ah, right, good point. Pointing that out in the commit message might be a good idea; otherwise people may miss that ;)
Quoted 6 lines
Though it's quite a churn across the drivers I think extra verbosity here is justified. I'd only suggest to print stable things. Like
prog->aux->idprobably has little value for the person looking at the logs. That prog id is likely gone. If it wasprog->aux->nameit would be more helpful.
Same withexpected_attach_type. Why print it at all?
tracepoint is probably good idea too.Thanks for the feedback.
I tried to select the additional arguments to allow the user/admin tracking down which program is causing the issue and why. I'm a complete newbe wrt XDP, so likely my choice were naive.
I thought the id identifies the program in an unambiguous manner. I understand the program could be unloaded meanwhile, but if that is not the case the id should be quite useful. Perhaps we could dump both the id and the name?
Quoted 4 lines
I included the attach type as different types support/allow different
actions: the same program could cause the warning or not depending on it. If that is not useful I can drop the attach type from the next iteration.
The attach type identifies DEVMAP and CPUMAP programs, but just printing it as a number probably doesn't make sense. So maybe something like:
switch(prog->expected_attach_type) {
case BPF_XDP_DEVMAP:
case BPF_XDP_CPUMAP:
pr_warn_once("Illegal XDP return value %u from prog %s(%d) in %s!\n",
act, prog->aux_name, prog->aux->id,
prog->expected_attach_type == BPF_XDP_DEVMAP ? "devmap" : "cpumap");
break;
default:
pr_warn_once("%s XDP return value %u on prog %s(%d) dev %s, expect packet loss!\n",
act > act_max ? "Illegal" : "Driver unsupported",
act, prog->aux->name, prog->aux->id, dev->name);
break;
} -Toke
Re: [RFC PATCH 2/2] bpf: let bpf_warn_invalid_xdp_action() report more info
Quoted 55 lines · Toke Høiland-Jørgensen
Quoted 45 lines · Paolo Abeni
Hello,
Quoted 24 lines · Alexei Starovoitov
Quoted 16 lines · Toke Høiland-Jørgensen
Quoted 14 lines · Paolo Abeni
Quoted 10 lines
Review
Quoted 5 lines
(quoted diff) +2
-2- pr_warn_once("%s XDP return value %u, expect packet loss!\n", + pr_warn_once("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n", act > act_max ? "Illegal" : "Driver unsupported", - act); + act, prog->aux->id, dev->name, prog->expected_attach_type);This will only ever trigger once per reboot even if the message differs, right? Which makes it less useful as a debugging aid; so I'm not sure if it's really worth it with this intrusive change unless we also do something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general agreement about this change.
Quoted 6 lines
Quoted 3 lines
I think this patch is needed because the
WARN_ONCEsplat gives implicitly information about the related driver and attach type. Replacing with a simple printk we lose them.Ah, right, good point. Pointing that out in the commit message might be a good idea; otherwise people may miss that ;)
Quoted 6 lines
Though it's quite a churn across the drivers I think extra verbosity here is justified. I'd only suggest to print stable things. Like
prog->aux->idprobably has little value for the person looking at the logs. That prog id is likely gone. If it wasprog->aux->nameit would be more helpful.
Same withexpected_attach_type. Why print it at all?
tracepoint is probably good idea too.Thanks for the feedback.
I tried to select the additional arguments to allow the user/admin tracking down which program is causing the issue and why. I'm a complete newbe wrt XDP, so likely my choice were naive.
I thought the id identifies the program in an unambiguous manner. I understand the program could be unloaded meanwhile, but if that is not the case the id should be quite useful. Perhaps we could dump both the id and the name?
Quoted 4 lines
I included the attach type as different types support/allow different
actions: the same program could cause the warning or not depending on it. If that is not useful I can drop the attach type from the next iteration.The attach type identifies DEVMAP and CPUMAP programs, but just printing it as a number probably doesn't make sense. So maybe something like:
Quoted 12 lines
switch(prog->expected_attach_type) {
case BPF_XDP_DEVMAP:
case BPF_XDP_CPUMAP:
pr_warn_once("Illegal XDP return value %u from prog %s(%d) in %s!\n",
act, prog->aux_name, prog->aux->id,
prog->expected_attach_type == BPF_XDP_DEVMAP ? "devmap" : "cpumap");
break;
default:
pr_warn_once("%s XDP return value %u on prog %s(%d) dev %s, expect packet loss!\n",
act > act_max ? "Illegal" : "Driver unsupported",
act, prog->aux->name, prog->aux->id, dev->name);
break;imo it's an overkill for a debug message that should not be seen. Just attach bpf probe to this function and print whatever necessary. Keep kernel message single line and short that is easily greppable.