| Message ID | 20230418002917.519492-1-kuifeng@meta.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpftool: Show map IDs along with struct_ops links. | expand |
On Tue, 18 Apr 2023 at 01:29, Kui-Feng Lee <thinker.li@gmail.com> wrote: > > A new link type, BPF_LINK_TYPE_STRUCT_OPS, was added to attach > struct_ops to links. (226bc6ae6405) It would be helpful for users to > know which map is associated with the link. > > The assumption was that every link is associated with a BPF program, > but this does not hold true for struct_ops. It would be better to > display map_id instead of prog_id for struct_ops links. However, some > tools may rely on the old assumption and need a prog_id displayed in > the link header line. By keeping the prog_id unchanged, an extra line > indicating the map_id is displayed. > > Signed-off-by: Kui-Feng Lee <kuifeng@meta.com> Reviewed-by: Quentin Monnet <quentin@isovalent.com> Thanks! What does the prog_id correspond to, for this type of links? If it's not relevant at all we could at least take it out from the plain output maybe, tools that want to parse the output should stick to JSON.
On 4/18/23 14:44, Quentin Monnet wrote: > On Tue, 18 Apr 2023 at 01:29, Kui-Feng Lee <thinker.li@gmail.com> wrote: >> >> A new link type, BPF_LINK_TYPE_STRUCT_OPS, was added to attach >> struct_ops to links. (226bc6ae6405) It would be helpful for users to >> know which map is associated with the link. >> >> The assumption was that every link is associated with a BPF program, >> but this does not hold true for struct_ops. It would be better to >> display map_id instead of prog_id for struct_ops links. However, some >> tools may rely on the old assumption and need a prog_id displayed in >> the link header line. By keeping the prog_id unchanged, an extra line >> indicating the map_id is displayed. >> >> Signed-off-by: Kui-Feng Lee <kuifeng@meta.com> > > Reviewed-by: Quentin Monnet <quentin@isovalent.com> > > Thanks! What does the prog_id correspond to, for this type of links? > If it's not relevant at all we could at least take it out from the > plain output maybe, tools that want to parse the output should stick > to JSON. The prog_id is irrelevant here. Since our convention is to let tools parse JSON, I will move map_id to the header line of plain text output and remove the prog_id, just like you said.
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c index f985b79cca27..cb41a447ae3a 100644 --- a/tools/bpf/bpftool/link.c +++ b/tools/bpf/bpftool/link.c @@ -195,6 +195,10 @@ static int show_link_close_json(int fd, struct bpf_link_info *info) info->netns.netns_ino); show_link_attach_type_json(info->netns.attach_type, json_wtr); break; + case BPF_LINK_TYPE_STRUCT_OPS: + jsonw_uint_field(json_wtr, "map_id", + info->struct_ops.map_id); + break; default: break; } @@ -301,6 +305,9 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info) printf("\n\tnetns_ino %u ", info->netns.netns_ino); show_link_attach_type_plain(info->netns.attach_type); break; + case BPF_LINK_TYPE_STRUCT_OPS: + printf("\n\tmap_id %u ", info->struct_ops.map_id); + break; default: break; }
A new link type, BPF_LINK_TYPE_STRUCT_OPS, was added to attach struct_ops to links. (226bc6ae6405) It would be helpful for users to know which map is associated with the link. The assumption was that every link is associated with a BPF program, but this does not hold true for struct_ops. It would be better to display map_id instead of prog_id for struct_ops links. However, some tools may rely on the old assumption and need a prog_id displayed in the link header line. By keeping the prog_id unchanged, an extra line indicating the map_id is displayed. Signed-off-by: Kui-Feng Lee <kuifeng@meta.com> --- tools/bpf/bpftool/link.c | 7 +++++++ 1 file changed, 7 insertions(+)