Re: [PATCH v5 2/3] media: rc: introduce BPF_PROG_LIRC_MODE2

[Daniel Borkmann]

On 06/05/2018 12:16 PM, Sean Young wrote:
> On Mon, Jun 04, 2018 at 07:47:30PM +0200, Matthias Reichl wrote:
[...]
>>> @@ -1695,6 +1700,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
>>>  	case BPF_CGROUP_SOCK_OPS:
>>>  	case BPF_CGROUP_DEVICE:
>>>  		break;
>>> +	case BPF_LIRC_MODE2:
>>> +		return lirc_prog_query(attr, uattr);
>>
>> When testing this patch series I was wondering why I always got
>> -EINVAL when trying to query the registered programs.
>>
>> Closer inspection revealed that bpf_prog_attach/detach/query and
>> calls to them in the bpf syscall are in "#ifdef CONFIG_CGROUP_BPF"
>> blocks - and as I built the kernel without CONFIG_CGROUP_BPF
>> BPF_PROG_ATTACH/DETACH/QUERY weren't handled in the syscall switch
>> and I got -EINVAL from the bpf syscall function.
>>
>> I haven't checked in detail yet, but it looks to me like
>> bpf_prog_attach/detach/query could always be built (or when
>> either cgroup bpf or lirc bpf are enabled) and the #ifdefs moved
>> inside the switch(). So lirc bpf could be used without cgroup bpf.
>> Or am I missing something?
> 
> You are right, this features depends on CONFIG_CGROUP_BPF right now. This
> also affects the BPF_SK_MSG_VERDICT, BPF_SK_SKB_STREAM_VERDICT and
> BPF_SK_SKB_STREAM_PARSER type bpf attachments, and as far as I know
> these shouldn't depend on CONFIG_CGROUP_BPF either.

The latter three do depend on it from a bigger picture as sockmap progs
are orchestrated via cgroups. But I'd be fine if you decouple lirc from
it since there's really no dependency at all, and presumably there are
valid cases where you would want to run it on low-end devices with minimal
kernel.

Thanks,
Daniel