| Message ID | a31b2599948c8d8679c6454b9191e70c1c732c32.1616179034.git.lorenzo@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mvneta: introduce XDP multi-buffer support | expand |
On 20 Mar 2021, at 4:42, David Ahern wrote: > On 3/19/21 3:47 PM, Lorenzo Bianconi wrote: >> diff --git a/include/net/xdp.h b/include/net/xdp.h >> index 19cd6642e087..e47d9e8da547 100644 >> --- a/include/net/xdp.h >> +++ b/include/net/xdp.h >> @@ -75,6 +75,10 @@ struct xdp_buff { >> struct xdp_txq_info *txq; >> u32 frame_sz:31; /* frame size to deduce data_hard_end/reserved >> tailroom*/ >> u32 mb:1; /* xdp non-linear buffer */ >> + u32 frame_length; /* Total frame length across all buffers. Only >> needs >> + * to be updated by helper functions, as it will be >> + * initialized at XDP program start. >> + */ >> }; >> >> static __always_inline void > > If you do another version of this set ... > > I think you only need 17-bits for the frame length (size is always <= > 128kB). It would be helpful for extensions to xdp if you annotated how > many bits are really needed here. Guess this can be done, but I did not too avoid the use of constants to do the BPF extraction. Here is an example of what might need to be added, as adding them before made people unhappy ;) https://elixir.bootlin.com/linux/v5.12-rc4/source/include/linux/skbuff.h#L801
On 3/22/21 3:29 AM, Eelco Chaudron wrote: > > > On 20 Mar 2021, at 4:42, David Ahern wrote: > >> On 3/19/21 3:47 PM, Lorenzo Bianconi wrote: >>> diff --git a/include/net/xdp.h b/include/net/xdp.h >>> index 19cd6642e087..e47d9e8da547 100644 >>> --- a/include/net/xdp.h >>> +++ b/include/net/xdp.h >>> @@ -75,6 +75,10 @@ struct xdp_buff { >>> struct xdp_txq_info *txq; >>> u32 frame_sz:31; /* frame size to deduce data_hard_end/reserved >>> tailroom*/ >>> u32 mb:1; /* xdp non-linear buffer */ >>> + u32 frame_length; /* Total frame length across all buffers. Only >>> needs >>> + * to be updated by helper functions, as it will be >>> + * initialized at XDP program start. >>> + */ >>> }; >>> >>> static __always_inline void >> >> If you do another version of this set ... >> >> I think you only need 17-bits for the frame length (size is always <= >> 128kB). It would be helpful for extensions to xdp if you annotated how >> many bits are really needed here. > > Guess this can be done, but I did not too avoid the use of constants to > do the BPF extraction. > Here is an example of what might need to be added, as adding them before > made people unhappy ;) > > https://elixir.bootlin.com/linux/v5.12-rc4/source/include/linux/skbuff.h#L801 > > I was just referring to a code comment that bits can be taken from frame_length for extensions - just like the mb bit takes from frame_sz (and it too has bits available since 2GB is way larger than actually needed).
diff --git a/include/linux/filter.h b/include/linux/filter.h index b2b85b2cad8e..511d812fd18c 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -768,6 +768,13 @@ static __always_inline u32 bpf_prog_run_xdp(const struct bpf_prog *prog, * already takes rcu_read_lock() when fetching the program, so * it's not necessary here anymore. */ + xdp->frame_length = xdp->data_end - xdp->data; + if (unlikely(xdp->mb)) { + struct xdp_shared_info *xdp_sinfo; + + xdp_sinfo = xdp_get_shared_info_from_buff(xdp); + xdp->frame_length += xdp_sinfo->data_length; + } return __BPF_PROG_RUN(prog, xdp, BPF_DISPATCHER_FUNC(xdp)); } diff --git a/include/net/xdp.h b/include/net/xdp.h index 19cd6642e087..e47d9e8da547 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -75,6 +75,10 @@ struct xdp_buff { struct xdp_txq_info *txq; u32 frame_sz:31; /* frame size to deduce data_hard_end/reserved tailroom*/ u32 mb:1; /* xdp non-linear buffer */ + u32 frame_length; /* Total frame length across all buffers. Only needs + * to be updated by helper functions, as it will be + * initialized at XDP program start. + */ }; static __always_inline void @@ -235,6 +239,14 @@ void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp) xdp->data_meta = frame->data - frame->metasize; xdp->frame_sz = frame->frame_sz; xdp->mb = frame->mb; + xdp->frame_length = frame->len; + + if (unlikely(xdp->mb)) { + struct xdp_shared_info *xdp_sinfo; + + xdp_sinfo = xdp_get_shared_info_from_buff(xdp); + xdp->frame_length += xdp_sinfo->data_length; + } } static inline diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 2d3036e292a9..4dcc5ad736b4 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5213,6 +5213,7 @@ struct xdp_md { __u32 rx_queue_index; /* rxq->queue_index */ __u32 egress_ifindex; /* txq->dev->ifindex */ + __u32 frame_length; }; /* DEVMAP map-value layout diff --git a/net/core/filter.c b/net/core/filter.c index a607ea8321bd..b047757bd839 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3873,6 +3873,7 @@ static int bpf_xdp_mb_adjust_tail(struct xdp_buff *xdp, int offset) memset(xdp_get_frag_address(frag) + size, 0, offset); xdp_set_frag_size(frag, size + offset); xdp_sinfo->data_length += offset; + xdp->frame_length += offset; } else { int i, frags_to_free = 0; @@ -3894,6 +3895,7 @@ static int bpf_xdp_mb_adjust_tail(struct xdp_buff *xdp, int offset) * to adjust the data_length in line. */ xdp_sinfo->data_length -= shrink; + xdp->frame_length -= shrink; xdp_set_frag_size(frag, size - shrink); break; } @@ -9126,6 +9128,12 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type, *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg, offsetof(struct net_device, ifindex)); break; + case offsetof(struct xdp_md, frame_length): + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_buff, + frame_length), + si->dst_reg, si->src_reg, + offsetof(struct xdp_buff, frame_length)); + break; } return insn - insn_buf; diff --git a/net/core/xdp.c b/net/core/xdp.c index 7388bc6d680b..fb7d0724a5b6 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -510,6 +510,7 @@ void xdp_return_num_frags_from_buff(struct xdp_buff *xdp, u16 num_frags) struct page *page = xdp_get_frag_page(frag); xdp_sinfo->data_length -= xdp_get_frag_size(frag); + xdp->frame_length -= xdp_get_frag_size(frag); __xdp_return(page_address(page), &xdp->rxq->mem, false, NULL); } xdp_sinfo->nr_frags -= num_frags; diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 2d3036e292a9..4dcc5ad736b4 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5213,6 +5213,7 @@ struct xdp_md { __u32 rx_queue_index; /* rxq->queue_index */ __u32 egress_ifindex; /* txq->dev->ifindex */ + __u32 frame_length; }; /* DEVMAP map-value layout