[PATCH] bpf: allow BPF programs access skb_shared_info->gso_segs field

[PATCH] bpf: allow BPF programs access skb_shared_info->gso_segs field

[Eddie Hao]

This adds the ability to read gso_segs from a BPF program. This is useful
for user space monitoring systems to get gso_segs statistics for each
arbitrary group of flows defined by the BPF program.

Signed-off-by: Eddie Hao <eddieh@google.com>
---
 include/uapi/linux/bpf.h | 1 +
 net/core/filter.c        | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b7db3261c62d..282085445616 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2266,6 +2266,7 @@ struct __sk_buff {
 	/* ... here. */
 
 	__u32 data_meta;
+	unsigned short gso_segs;
 };
 
 struct bpf_tunnel_key {
diff --git a/net/core/filter.c b/net/core/filter.c
index b9ec916f4e3a..f17e249f3fbb 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5820,6 +5820,15 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
 				      bpf_target_off(struct sock_common,
 						     skc_num, 2, target_size));
 		break;
+
+	case offsetof(struct __sk_buff, gso_segs):
+		BUILD_BUG_ON(FIELD_SIZEOF(struct skb_shared_info, gso_segs) !=
+					  sizeof(unsigned short));
+		off = offsetof(struct sk_buff, end);
+		off += offsetof(struct skb_shared_info, gso_segs);
+		*insn++ = BPF_LDX_MEM(BPF_SIZEOF(unsigned short), si->dst_reg,
+						 si->src_reg, off);
+		break;
 	}
 
 	return insn - insn_buf;
-- 
2.18.0.203.gfac676dfb9-goog

Re: [PATCH] bpf: allow BPF programs access skb_shared_info->gso_segs field

[Eddie Hao]

Sorry, forgot to annotate the subject line with bpf-next.

On Sun, Jul 15, 2018 at 1:45 AM Eddie Hao <eddieh@google.com> wrote:
>
> This adds the ability to read gso_segs from a BPF program. This is useful
> for user space monitoring systems to get gso_segs statistics for each
> arbitrary group of flows defined by the BPF program.
>
> Signed-off-by: Eddie Hao <eddieh@google.com>
> ---
>  include/uapi/linux/bpf.h | 1 +
>  net/core/filter.c        | 9 +++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b7db3261c62d..282085445616 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2266,6 +2266,7 @@ struct __sk_buff {
>         /* ... here. */
>
>         __u32 data_meta;
> +       unsigned short gso_segs;
>  };
>
>  struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b9ec916f4e3a..f17e249f3fbb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5820,6 +5820,15 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>                                       bpf_target_off(struct sock_common,
>                                                      skc_num, 2, target_size));
>                 break;
> +
> +       case offsetof(struct __sk_buff, gso_segs):
> +               BUILD_BUG_ON(FIELD_SIZEOF(struct skb_shared_info, gso_segs) !=
> +                                         sizeof(unsigned short));
> +               off = offsetof(struct sk_buff, end);
> +               off += offsetof(struct skb_shared_info, gso_segs);
> +               *insn++ = BPF_LDX_MEM(BPF_SIZEOF(unsigned short), si->dst_reg,
> +                                                si->src_reg, off);
> +               break;
>         }
>
>         return insn - insn_buf;
> --
> 2.18.0.203.gfac676dfb9-goog
>

Re: [PATCH] bpf: allow BPF programs access skb_shared_info->gso_segs field

[Alexei Starovoitov]

On Sun, Jul 15, 2018 at 01:45:24AM -0700, Eddie Hao wrote:
> This adds the ability to read gso_segs from a BPF program. This is useful
> for user space monitoring systems to get gso_segs statistics for each
> arbitrary group of flows defined by the BPF program.
> 
> Signed-off-by: Eddie Hao <eddieh@google.com>
> ---
>  include/uapi/linux/bpf.h | 1 +
>  net/core/filter.c        | 9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b7db3261c62d..282085445616 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2266,6 +2266,7 @@ struct __sk_buff {
>  	/* ... here. */
>  
>  	__u32 data_meta;
> +	unsigned short gso_segs;

'short' will make it harder to extend due to struct hole.
please use __u32 instead.

>  };
>  
>  struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b9ec916f4e3a..f17e249f3fbb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5820,6 +5820,15 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>  				      bpf_target_off(struct sock_common,
>  						     skc_num, 2, target_size));
>  		break;
> +
> +	case offsetof(struct __sk_buff, gso_segs):
> +		BUILD_BUG_ON(FIELD_SIZEOF(struct skb_shared_info, gso_segs) !=
> +					  sizeof(unsigned short));
> +		off = offsetof(struct sk_buff, end);
> +		off += offsetof(struct skb_shared_info, gso_segs);

did you test this patch?
Above math doesn't look correct.
NET_SKBUFF_DATA_USES_OFFSET is not handled. At least one more LDX is needed.

> +		*insn++ = BPF_LDX_MEM(BPF_SIZEOF(unsigned short), si->dst_reg,
> +						 si->src_reg, off);

I think BUILD_BUG_ON is a bit of overkill when BPF_LDX_MEM
can use BPF_FIELD_SIZEOF(struct skb_shared_info, gso_segs) instead.