netdev - Re: [PATCH net-next 1/2] gre: add sequence number for collect md mode.

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:   Thu, 1 Mar 2018 09:17:52 -0800
From:   William Tu <u9012063@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH net-next 1/2] gre: add sequence number for collect md mode.

On Thu, Mar 1, 2018 at 2:18 AM, Daniel Borkmann <daniel@...earbox.net> wrote:
> On 03/01/2018 01:11 AM, William Tu wrote:
>> Currently GRE sequence number can only be used in native
>> tunnel mode.  This patch adds sequence number support for
>> gre collect metadata mode.  RFC2890 defines GRE sequence
>> number to be specific to the traffic flow identified by the
>> key.  However, this patch does not implement per-key seqno.
>> The sequence number is shared in the same tunnel device.
>> That is, different tunnel keys using the same collect_md
>> tunnel share single sequence number.
>>
>> Signed-off-by: William Tu <u9012063@...il.com>
>> ---
>>  include/uapi/linux/bpf.h |  1 +
>>  net/core/filter.c        |  4 +++-
>>  net/ipv4/ip_gre.c        |  7 +++++--
>>  net/ipv6/ip6_gre.c       | 13 ++++++++-----
>>  4 files changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index db6bdc375126..2c6dd942953d 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -800,6 +800,7 @@ enum bpf_func_id {
>>  /* BPF_FUNC_skb_set_tunnel_key flags. */
>>  #define BPF_F_ZERO_CSUM_TX           (1ULL << 1)
>>  #define BPF_F_DONT_FRAGMENT          (1ULL << 2)
>> +#define BPF_F_GRE_SEQ                        (1ULL << 3)
>>
>>  /* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
>>   * BPF_FUNC_perf_event_read_value flags.
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 0c121adbdbaa..010305e0791a 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -2991,7 +2991,7 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
>>       struct ip_tunnel_info *info;
>>
>>       if (unlikely(flags & ~(BPF_F_TUNINFO_IPV6 | BPF_F_ZERO_CSUM_TX |
>> -                            BPF_F_DONT_FRAGMENT)))
>> +                            BPF_F_DONT_FRAGMENT | BPF_F_GRE_SEQ)))
>>               return -EINVAL;
>>       if (unlikely(size != sizeof(struct bpf_tunnel_key))) {
>>               switch (size) {
>> @@ -3025,6 +3025,8 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
>>               info->key.tun_flags |= TUNNEL_DONT_FRAGMENT;
>>       if (flags & BPF_F_ZERO_CSUM_TX)
>>               info->key.tun_flags &= ~TUNNEL_CSUM;
>> +     if (flags & BPF_F_GRE_SEQ)
>> +             info->key.tun_flags |= TUNNEL_SEQ;
>
> Ok, looks fine. My only minor request would be to rename BPF_F_GRE_SEQ
> into e.g. BPF_F_SEQ_NUMBER to at least not have something GRE specific
> in the name in case we could later on reuse it elsewhere as well, and
> the bpf_skb_set_tunnel_key() is unaware of the underlying encap anyway.

OK, make sense. Thanks!
I will rename it in the next version.
William