Re: [PATCH RFC bpf-next 0/4] bpf: Add support for XDP programs in DEVMAPs

[Daniel Borkmann <daniel@iogearbox.net>]

On 5/25/20 2:56 PM, Toke Høiland-Jørgensen wrote:
> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>> On Mon, 25 May 2020 14:15:32 +0200
>> Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>> David Ahern <dsahern@gmail.com> writes:
>>>> On 5/22/20 9:59 AM, Toke Høiland-Jørgensen wrote:
>>>>> David Ahern <dsahern@kernel.org> writes:
>>>>>    
>>>>>> Implementation of Daniel's proposal for allowing DEVMAP entries to be
>>>>>> a device index, program id pair. Daniel suggested an fd to specify the
>>>>>> program, but that seems odd to me that you insert the value as an fd, but
>>>>>> read it back as an id since the fd can be closed.
>>>>>
>>>>> While I can be sympathetic to the argument that it seems odd, every
>>>>> other API uses FD for insert and returns ID, so why make it different
>>>>> here? Also, the choice has privilege implications, since the CAP_BPF
>>>>> series explicitly makes going from ID->FD a more privileged operation
>>>>> than just querying the ID.

[...]

>> I sympathize with Ahern on this.  It seems very weird to insert/write
>> one value-type, but read another value-type.
> 
> Yeah, I do kinda agree that it's a bit weird. But it's what we do
> everywhere else, so I think consistency wins out here. There might be an
> argument that maps should be different (because they're conceptually a
> read/write data structure not a syscall return value). But again, we
> already have a map type that takes prog IDs, and that already does the
> rewriting, so doing it different here would be even weirder...

Sorry for the late reply. Agree, it would at least be consistent to what is done
in tail call maps, and the XDP netlink API where you have the fd->id in both cases.
Either way, quick glance over the patches, the direction of this RFC looks good to
me, better fit than the prior XDP egress approaches.

Thanks,
Daniel