On Sun, Jan 14, 2018 at 3:37 PM, Daniel Borkmann <dan...@iogearbox.net> wrote:
> Hi Jakub,
>
> Series looks fine, just stumbled over one small thing here below.
>
> On 01/12/2018 05:29 AM, Jakub Kicinski wrote:
> [...]
>> +bool bpf_offload_dev_match(struct bpf_prog *prog, struct bpf_map *map)
>> +{
>> + struct bpf_offloaded_map *offmap;
>> + struct bpf_prog_offload *offload;
>> + bool ret;
>> +
>> + if (!!bpf_prog_is_dev_bound(prog->aux) != !!bpf_map_is_dev_bound(map))
>> + return false;
>> + if (!bpf_prog_is_dev_bound(prog->aux))
>> + return true;
>
> Should this not say 'false' if the prog has no offload_requested ...
>
>> + down_read(&bpf_devs_lock);
>> + offload = prog->aux->offload;
>> + offmap = map_to_offmap(map);
>> +
>> + ret = offload && offload->netdev == offmap->netdev;
>
> ... meaning we return true from bpf_offload_dev_match() only in the
> case when netdevs match?
IOW return false when both program and map are not offloaded? I was
going for "are those two compatible" kind of logic.
But I'll change, the only user of this function is the verifier
compatibility check and that already handles the "neither is
offloaded" case.
>> + up_read(&bpf_devs_lock);
>> +
>> + return ret;
>> +}
> Thanks,
> Daniel
