netdev - Re: [PATCH bpf-next 09/13] bpf: add bpf_skb_adjust

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Mar 2019 20:13:31 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     netdev@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        sdf@...gle.com, posk@...gle.com,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH bpf-next 09/13] bpf: add bpf_skb_adjust_room encap flags

On Wed, Mar 20, 2019 at 10:49:40AM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@...gle.com>
> 
> When pushing tunnel headers, annotate skbs in the same way as tunnel
> devices.
> 
> For GSO packets, the network stack requires certain fields set to
> segment packets with tunnel headers. gro_gse_segment depends on
> transport and inner mac header, for instance.
> 
> Add an option to pass this information.
> 
> Remove the restriction on len_diff to network header length, which
> is too short, e.g., for GRE protocols.
> 
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> ---
>  include/uapi/linux/bpf.h | 14 +++++++++-
>  net/core/filter.c        | 58 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 67 insertions(+), 5 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0eda8f564a381..a444534cc88d7 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2595,7 +2595,19 @@ enum bpf_func_id {
>  
>  /* BPF_FUNC_skb_adjust_room flags. */
>  #define BPF_F_ADJ_ROOM_FIXED_GSO	(1ULL << 0)
> -#define BPF_F_ADJ_ROOM_MASK		(BPF_F_ADJ_ROOM_FIXED_GSO)
> +
> +#define BPF_F_ADJ_ROOM_ENCAP_L3_IPV4	(1ULL << 1)
> +#define BPF_F_ADJ_ROOM_ENCAP_L3_IPV6	(1ULL << 2)
> +#define BPF_F_ADJ_ROOM_ENCAP_L3_MASK	(BPF_F_ADJ_ROOM_ENCAP_L3_IPV4 | \
> +					 BPF_F_ADJ_ROOM_ENCAP_L3_IPV6)
> +
> +#define BPF_F_ADJ_ROOM_ENCAP_L4_GRE	(1ULL << 3)
> +#define BPF_F_ADJ_ROOM_ENCAP_L4_UDP	(1ULL << 4)
> +
> +#define BPF_F_ADJ_ROOM_MASK		(BPF_F_ADJ_ROOM_FIXED_GSO | \
> +					 BPF_F_ADJ_ROOM_ENCAP_L3_MASK | \
> +					 BPF_F_ADJ_ROOM_ENCAP_L4_GRE | \
> +					 BPF_F_ADJ_ROOM_ENCAP_L4_UDP)

The patches looks great.
One small nit. Please move *_MASK out of uapi.
When flags are added this uapi macro keeps changing
which may break user space compiled with older macro.
Essentially user space cannot rely on this macro and cannot use it.
I think it's best to leave this macro for kernel internals.