netdev - Re: [bpf-next PATCH v2] bpf: Add rx_queue_mapping to bpf

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] [day] [month] [year] [list]

Message-ID: <c2aa1fe8-606c-bee7-672d-2fcfed86a263@intel.com>
Date:   Tue, 26 May 2020 17:27:57 -0700
From:   "Nambiar, Amritha" <amritha.nambiar@...el.com>
To:     Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        davem@...emloft.net, ast@...nel.org
Cc:     kafai@...com, sridhar.samudrala@...el.com
Subject: Re: [bpf-next PATCH v2] bpf: Add rx_queue_mapping to bpf_sock

On 5/22/2020 4:34 PM, Daniel Borkmann wrote:
> On 5/22/20 8:28 PM, Amritha Nambiar wrote:
>> Add "rx_queue_mapping" to bpf_sock. This gives read access for the
>> existing field (sk_rx_queue_mapping) of struct sock from bpf_sock.
>> Semantics for the bpf_sock rx_queue_mapping access are similar to
>> sk_rx_queue_get(), i.e the value NO_QUEUE_MAPPING is not allowed
>> and -1 is returned in that case.
> 
> This adds the "what this patch does" but could you also add a 
> description for
> the use-case in here?
> 

Will fix in v3.

>> v2: fixed build error for CONFIG_XPS wrapping, reported by
>>      kbuild test robot <lkp@...el.com>
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@...el.com>
>> ---
>>   include/uapi/linux/bpf.h |    1 +
>>   net/core/filter.c        |   18 ++++++++++++++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 97e1fd19ff58..d2acd5aeae8d 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -3530,6 +3530,7 @@ struct bpf_sock {
>>       __u32 dst_ip4;
>>       __u32 dst_ip6[4];
>>       __u32 state;
>> +    __u32 rx_queue_mapping;
>>   };
>>   struct bpf_tcp_sock {
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index bd2853d23b50..c4ba92204b73 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -6829,6 +6829,7 @@ bool bpf_sock_is_valid_access(int off, int size, 
>> enum bpf_access_type type,
>>       case offsetof(struct bpf_sock, protocol):
>>       case offsetof(struct bpf_sock, dst_port):
>>       case offsetof(struct bpf_sock, src_port):
>> +    case offsetof(struct bpf_sock, rx_queue_mapping):
>>       case bpf_ctx_range(struct bpf_sock, src_ip4):
>>       case bpf_ctx_range_till(struct bpf_sock, src_ip6[0], src_ip6[3]):
>>       case bpf_ctx_range(struct bpf_sock, dst_ip4):
>> @@ -7872,6 +7873,23 @@ u32 bpf_sock_convert_ctx_access(enum 
>> bpf_access_type type,
>>                               skc_state),
>>                          target_size));
>>           break;
>> +    case offsetof(struct bpf_sock, rx_queue_mapping):
>> +#ifdef CONFIG_XPS
>> +        *insn++ = BPF_LDX_MEM(
>> +            BPF_FIELD_SIZEOF(struct sock, sk_rx_queue_mapping),
>> +            si->dst_reg, si->src_reg,
>> +            bpf_target_off(struct sock, sk_rx_queue_mapping,
>> +                       sizeof_field(struct sock,
>> +                            sk_rx_queue_mapping),
>> +                       target_size));
>> +        *insn++ = BPF_JMP_IMM(BPF_JNE, si->dst_reg, NO_QUEUE_MAPPING,
>> +                      1);
>> +        *insn++ = BPF_MOV64_IMM(si->dst_reg, -1);
>> +#else
>> +        *insn++ = BPF_MOV64_IMM(si->dst_reg, 0);
> 
> This should be -1 as queue mapping as well if XPS is not configured, no?
> Otherwise, how do you tell it apart from an actual mapping to 0 if XPS is
> built-in?
> 

Agree. My bad. Will fix in v3.

>> +        *target_size = 2;
>> +#endif
>> +        break;
>>       }
>>       return insn - insn_buf;
>>
>