Message ID | 20240305202155.3890667-1-yonghong.song@linux.dev (mailing list archive) |
---|---|
Headers | show |
Series | Add bpf_link support for sk_msg prog | expand |
Yonghong Song wrote: > One of our internal services started to use sk_msg program and currently > it used existing prog attach/detach2 as demonstrated in selftests. > But attach/detach of all other bpf programs are based on bpf_link. > Consistent attach/detach APIs for all programs will make things easy to > undersand and less error prone. So this patch added bpf_link > support for BPF_PROG_TYPE_SK_MSG. > > I marked the patch as RFC as not all functionality are covered > by tests yet, e.g. update_prog(). Maybe somebody can suggest > an existing test which I can look into. > Or maybe some other tests need to be added as well. Agree this was missing it will be nice to get this merged. The links are much nicer to deal with.
On Tue, Mar 05, 2024 at 12:21 PM -08, Yonghong Song wrote: > One of our internal services started to use sk_msg program and currently > it used existing prog attach/detach2 as demonstrated in selftests. > But attach/detach of all other bpf programs are based on bpf_link. > Consistent attach/detach APIs for all programs will make things easy to > undersand and less error prone. So this patch added bpf_link > support for BPF_PROG_TYPE_SK_MSG. > > I marked the patch as RFC as not all functionality are covered > by tests yet, e.g. update_prog(). Maybe somebody can suggest > an existing test which I can look into. > Or maybe some other tests need to be added as well. I have a general remark, not specific to this work. We can't attach with links from CLI, apart from when auto-attach is supported. `bpftool prog attach` doesn't use bpf links. For instance: bpftool prog attach \ pinned /sys/fs/bpf/test/sk_msg_prog \ sk_msg_verdict \ pinned /sys/fs/bpf/test/sock_map Is there a plan for the CLI tooling to support it?
On 3/6/24 11:19 AM, John Fastabend wrote: > Yonghong Song wrote: >> One of our internal services started to use sk_msg program and currently >> it used existing prog attach/detach2 as demonstrated in selftests. >> But attach/detach of all other bpf programs are based on bpf_link. >> Consistent attach/detach APIs for all programs will make things easy to >> undersand and less error prone. So this patch added bpf_link >> support for BPF_PROG_TYPE_SK_MSG. >> >> I marked the patch as RFC as not all functionality are covered >> by tests yet, e.g. update_prog(). Maybe somebody can suggest >> an existing test which I can look into. >> Or maybe some other tests need to be added as well. > Agree this was missing it will be nice to get this merged. The links > are much nicer to deal with. Agree that bpf_link is better as proved by other cases. It would be great if you can take a look at the patch set and let me any issues, missing cases, missing tests, etc. Thanks!
On Tue, Mar 05, 2024 at 12:21 PM -08, Yonghong Song wrote: [...] > I marked the patch as RFC as not all functionality are covered > by tests yet, e.g. update_prog(). Maybe somebody can suggest > an existing test which I can look into. > Or maybe some other tests need to be added as well. Looks like we don't have prog update covered today. That would be something to add to sockmap_basic suite.
On 3/7/24 5:01 AM, Jakub Sitnicki wrote: > On Tue, Mar 05, 2024 at 12:21 PM -08, Yonghong Song wrote: >> One of our internal services started to use sk_msg program and currently >> it used existing prog attach/detach2 as demonstrated in selftests. >> But attach/detach of all other bpf programs are based on bpf_link. >> Consistent attach/detach APIs for all programs will make things easy to >> undersand and less error prone. So this patch added bpf_link >> support for BPF_PROG_TYPE_SK_MSG. >> >> I marked the patch as RFC as not all functionality are covered >> by tests yet, e.g. update_prog(). Maybe somebody can suggest >> an existing test which I can look into. >> Or maybe some other tests need to be added as well. > I have a general remark, not specific to this work. > > We can't attach with links from CLI, apart from when auto-attach is > supported. `bpftool prog attach` doesn't use bpf links. For instance: > > bpftool prog attach \ > pinned /sys/fs/bpf/test/sk_msg_prog \ > sk_msg_verdict \ > pinned /sys/fs/bpf/test/sock_map > > Is there a plan for the CLI tooling to support it? As far as I know, there is no plan to support this. Of course, somebody could try to implement this ...