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

[Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>]

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,
Toshiaki Makita