diff mbox series

[v7,bpf-next,10/14] bpf: add new frame_length field to the XDP ctx

Message ID a31b2599948c8d8679c6454b9191e70c1c732c32.1616179034.git.lorenzo@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series mvneta: introduce XDP multi-buffer support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 9 maintainers not CCed: joe@cilium.io linux-api@vger.kernel.org yhs@fb.com kpsingh@kernel.org hawk@kernel.org andrii@kernel.org kafai@fb.com songliubraving@fb.com quentin@isovalent.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 12261 this patch: 12261
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 84 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 12767 this patch: 12767
netdev/header_inline success Link

Commit Message

lorenzo@kernel.org March 19, 2021, 9:47 p.m. UTC 
From: Eelco Chaudron <echaudro@redhat.com>

This patch adds a new field to the XDP context called frame_length,
which will hold the full length of the packet, including fragments
if existing.

eBPF programs can determine if fragments are present using something
like:

  if (ctx->data_end - ctx->data < ctx->frame_length) {
    /* Fragements exists. /*
  }

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/linux/filter.h         |  7 +++++++
 include/net/xdp.h              | 12 ++++++++++++
 include/uapi/linux/bpf.h       |  1 +
 net/core/filter.c              |  8 ++++++++
 net/core/xdp.c                 |  1 +
 tools/include/uapi/linux/bpf.h |  1 +
 6 files changed, 30 insertions(+)

Comments

David Ahern March 20, 2021, 3:42 a.m. UTC | #1 
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.
Eelco Chaudron March 22, 2021, 9:29 a.m. UTC | #2 
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
David Ahern March 22, 2021, 3:05 p.m. UTC | #3 
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 mbox series

Patch

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