On XDP_PASS, when XDP runs on an skb, the stored values are lost, because the skb already exists. This happens in bpf_prog_run_generic_xdp() for generic XDP and for cpumap and devmap programs on an skb. It also happens in veth_xdp_rcv_skb() when veth receives an skb from its peer. veth has the skb-ext-pass feature, so bpf_prog_run_xdp() does not warn for it. Add xdp_skb_ext_warn_skb(). bpf_prog_run_generic_xdp() and veth_xdp_rcv_skb() call it on XDP_PASS with stored values, whatever the device features. Generic XDP also runs bpf_prog_run_xdp(). On a native XDP device without the feature, it would warn twice. So xdp_skb_ext_warn_pass() skips generic XDP. Signed-off-by: Tianyi Gao --- Tested on virtio_net and veth: generic XDP prints only the new warning, and BPF_PROG_RUN without live frames prints none. --- drivers/net/veth.c | 2 ++ include/net/xdp.h | 2 ++ net/core/dev.c | 2 ++ net/core/xdp.c | 21 ++++++++++++++++++++- 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index b402f8ed5096..f0d279e2da6a 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -826,6 +826,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, switch (act) { case XDP_PASS: + if (unlikely(xdp_buff_has_skb_ext(xdp))) + xdp_skb_ext_warn_skb(xdp_prog, xdp); break; case XDP_TX: veth_xdp_get(xdp); diff --git a/include/net/xdp.h b/include/net/xdp.h index 7148e8ef00e1..596261ed9cca 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -511,6 +511,8 @@ static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp) void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext); void xdp_skb_ext_warn_pass(const struct bpf_prog *prog, const struct xdp_buff *xdp); +void xdp_skb_ext_warn_skb(const struct bpf_prog *prog, + const struct xdp_buff *xdp); void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type, bool napi_direct, struct xdp_buff *xdp); diff --git a/net/core/dev.c b/net/core/dev.c index 3bf00e7e1a85..e37c881d968b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5705,6 +5705,8 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp, orig_eth_type = eth->h_proto; act = bpf_prog_run_xdp(xdp_prog, xdp); + if (unlikely(xdp_buff_has_skb_ext(xdp)) && act == XDP_PASS) + xdp_skb_ext_warn_skb(xdp_prog, xdp); /* check if bpf_xdp_adjust_head was used */ off = xdp->data - orig_data; diff --git a/net/core/xdp.c b/net/core/xdp.c index 36d6bb38b719..984299199906 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -18,6 +18,7 @@ #include #include +#include #include #include /* struct xdp_mem_allocator */ #include @@ -1124,12 +1125,17 @@ void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext) void xdp_skb_ext_warn_pass(const struct bpf_prog *prog, const struct xdp_buff *xdp) { - const struct net_device *dev = xdp->rxq->dev; + const struct xdp_rxq_info *rxq = xdp->rxq; + const struct net_device *dev = rxq->dev; /* The skb is built later from the xdp_frame */ if (prog->expected_attach_type == BPF_XDP_CPUMAP || prog->expected_attach_type == BPF_XDP_DEVMAP) return; + /* Generic XDP and BPF_PROG_RUN use the netdev_rx_queue xdp_rxq */ + if (rxq->queue_index < dev->num_rx_queues && + rxq == &dev->_rx[rxq->queue_index].xdp_rxq) + return; /* No native XDP, so only generic XDP or BPF_PROG_RUN runs on dev */ if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC)) return; @@ -1139,6 +1145,19 @@ void xdp_skb_ext_warn_pass(const struct bpf_prog *prog, } EXPORT_SYMBOL_GPL(xdp_skb_ext_warn_pass); +/** + * xdp_skb_ext_warn_skb - Warn that XDP_PASS on an skb loses the stored values. + * @prog: The XDP program that returned XDP_PASS. + * @xdp: The packet, with the XDP_FLAGS_HAS_SKB_EXT flag, built on an skb. + */ +void xdp_skb_ext_warn_skb(const struct bpf_prog *prog, + const struct xdp_buff *xdp) +{ + pr_warn_once("XDP_PASS on prog %s (id %u): dev %s runs XDP on an skb, the stored RX metadata is lost\n", + prog->aux->name, prog->aux->id, xdp->rxq->dev->name); +} +EXPORT_SYMBOL_GPL(xdp_skb_ext_warn_skb); + __bpf_kfunc_start_defs(); /** -- 2.56.0