netdev - Re: [PATCH bpf v2] xdp: Fix handling of devmap in generic XDP

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, 13 Jun 2018 21:36:40 -0700
From:   Y Song <ys114321@...il.com>
To:     Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        netdev <netdev@...r.kernel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Subject: Re: [PATCH bpf v2] xdp: Fix handling of devmap in generic XDP

On Wed, Jun 13, 2018 at 8:40 PM, Toshiaki Makita
<makita.toshiaki@....ntt.co.jp> wrote:
> On 2018/06/14 11:56, Y Song wrote:
> ...
>>> @@ -586,6 +589,15 @@ int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
>>>         return 0;
>>>  }
>>>
>>> +struct sk_buff;
>>> +
>>> +static inline int dev_map_generic_redirect(struct bpf_dtab_netdev *dst,
>>> +                                          struct sk_buff *skb,
>>> +                                          struct bpf_prog *xdp_prog)
>>> +{
>>> +       return 0;
>>
>> should you return an error code here?
>
> My understanding is that this function never be called if
> CONFIG_BPF_SYSCALL is not set so any value is OK.
> I aligned it with other functions of devmap, specifically
> dev_map_enqueue() which returns 0.
>
>>
>>> +}
>>> +
>>>  static inline
>>>  struct bpf_cpu_map_entry *__cpu_map_lookup_elem(struct bpf_map *map, u32 key)
>>>  {
>>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>>> index 45fc0f5..8ddff1f 100644
>>> --- a/include/linux/filter.h
>>> +++ b/include/linux/filter.h
>>> @@ -19,6 +19,7 @@
>>>  #include <linux/cryptohash.h>
>>>  #include <linux/set_memory.h>
>>>  #include <linux/kallsyms.h>
>>> +#include <linux/if_vlan.h>
>>>
>>>  #include <net/sch_generic.h>
>>>
>>> @@ -786,6 +787,21 @@ static inline bool bpf_dump_raw_ok(void)
>>>  struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
>>>                                        const struct bpf_insn *patch, u32 len);
>>>
>>> +static inline int __xdp_generic_ok_fwd_dev(struct sk_buff *skb,
>>> +                                          struct net_device *fwd)
>>
>> Previously this function is only used in filter.c and now it is only
>> used in devmap.c. Maybe this function should be in devmap.c
>> until it will be used cross different files?
>
> This function is also called from xdp_do_generic_redirect() in
> net/core/filter.c, so I can't move it to devmap.c.

Thanks for the explanation. Make sense.
Acked-by: Yonghong Song <yhs@...com>