Re: [PATCH bpf-next v2 07/15] bpf: offload: add map offload infrastructure

On Sun, Jan 14, 2018 at 3:37 PM, Daniel Borkmann [off-list ref] wrote:

Hi Jakub,

Series looks fine, just stumbled over one small thing here below.

On 01/12/2018 05:29 AM, Jakub Kicinski wrote:
[...]

quoted

+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 ...

quoted

+     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.

quoted

+     up_read(&bpf_devs_lock);
+
+     return ret;
+}

Thanks,
Daniel