netdev - Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

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]

Message-ID: <6d931d8e-41cf-7cbc-bedd-d0715527a499@gmail.com>
Date:   Wed, 22 Apr 2020 19:33:39 -0600
From:   David Ahern <dsahern@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        David Ahern <dsahern@...nel.org>
Cc:     Networking <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Prashant Bhole <prashantbhole.linux@...il.com>,
        Jason Wang <jasowang@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Toshiaki Makita <toshiaki.makita1@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        john fastabend <john.fastabend@...il.com>,
        Alexei Starovoitov <ast@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        David Ahern <dahern@...italocean.com>
Subject: Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

On 4/22/20 7:19 PM, Andrii Nakryiko wrote:
>>
>> +int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
>> +                         size_t info_size, __u32 flags)
>> +{
>> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
>> +                                      IFLA_XDP);
>> +}
>> +
>> +int bpf_get_link_xdp_egress_info(int ifindex, struct xdp_link_info *info,
>> +                                size_t info_size, __u32 flags)
>> +{
>> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
>> +                                      IFLA_XDP_EGRESS);
>> +}
>> +
>>  static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
>>  {
>>         if (info->attach_mode != XDP_ATTACHED_MULTI && !flags)
>> @@ -345,6 +376,22 @@ int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
>>         return ret;
>>  }
>>
>> +int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id, __u32 flags)
> 
> Is bpf_get_link_xdp_egress_id() even needed? This is a special case of
> bpf_get_link_xdp_egress_info(), I don't think we have to add another
> API to support it specifically.
> 
> Also, just curious, would it be better to have a generalized
> XDP/XDP_EGRESS xdp_info() functions instead of two separate ones?

Toke mentioned that too, and I have removed bpf_get_link_xdp_egress_info.

bpf_get_link_xdp_egress_id is needed. The flags argument is a direct
pass through for the kernel uapi and correlates to XDP_FLAGS that are
passed to the kernel. There is no harm or overhead in exporting symbols.
Having a simple, easy to use API for applications is important to making
a library user friendly which is key to its adoption.

Will fix the other comments.