netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types
Date: Fri, 30 Jul 2021 11:52:13 -0700	[thread overview]
Message-ID: <CAEf4BzYp2QgaL9ORzs0sWk6KO63Q-9ixU-vOsFfLckE3-bPg6A@mail.gmail.com> (raw)
In-Reply-To: <20210729162932.30365-4-quentin@isovalent.com>

On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> Update bpftool's list of attach type names to tell it about the latest
> attach types, or the "ringbuf" map. Also update the documentation, help
> messages, and bash completion when relevant.
>
> These missing items were reported by the newly added Python script used
> to help maintain consistency in bpftool.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>  .../bpftool/Documentation/bpftool-prog.rst    |  2 +-
>  tools/bpf/bpftool/bash-completion/bpftool     |  5 +-
>  tools/bpf/bpftool/common.c                    | 76 ++++++++++---------
>  tools/bpf/bpftool/prog.c                      |  4 +-
>  4 files changed, 47 insertions(+), 40 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index fe1b38e7e887..abf5f4cd7d3e 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -48,7 +48,7 @@ PROG COMMANDS
>  |              **struct_ops** | **fentry** | **fexit** | **freplace** | **sk_lookup**
>  |      }
>  |       *ATTACH_TYPE* := {
> -|              **msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
> +|              **msg_verdict** | **skb_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
>  |      }
>  |      *METRICs* := {
>  |              **cycles** | **instructions** | **l1d_loads** | **llc_misses**
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index b2e33a2d8524..69d018474537 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -405,7 +405,8 @@ _bpftool()
>                              ;;
>                          5)
>                              local BPFTOOL_PROG_ATTACH_TYPES='msg_verdict \
> -                                stream_verdict stream_parser flow_dissector'
> +                                skb_verdict stream_verdict stream_parser \
> +                                flow_dissector'
>                              COMPREPLY=( $( compgen -W \
>                                  "$BPFTOOL_PROG_ATTACH_TYPES" -- "$cur" ) )
>                              return 0
> @@ -708,7 +709,7 @@ _bpftool()
>                                  hash_of_maps devmap devmap_hash sockmap cpumap \
>                                  xskmap sockhash cgroup_storage reuseport_sockarray \
>                                  percpu_cgroup_storage queue stack sk_storage \
> -                                struct_ops inode_storage task_storage'
> +                                struct_ops inode_storage task_storage ringbuf'
>                              COMPREPLY=( $( compgen -W \
>                                  "$BPFTOOL_MAP_CREATE_TYPES" -- "$cur" ) )
>                              return 0
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 1828bba19020..b47797cac64f 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -31,42 +31,48 @@
>  #endif
>
>  const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
> -       [BPF_CGROUP_INET_INGRESS]       = "ingress",
> -       [BPF_CGROUP_INET_EGRESS]        = "egress",
> -       [BPF_CGROUP_INET_SOCK_CREATE]   = "sock_create",
> -       [BPF_CGROUP_INET_SOCK_RELEASE]  = "sock_release",
> -       [BPF_CGROUP_SOCK_OPS]           = "sock_ops",
> -       [BPF_CGROUP_DEVICE]             = "device",
> -       [BPF_CGROUP_INET4_BIND]         = "bind4",
> -       [BPF_CGROUP_INET6_BIND]         = "bind6",
> -       [BPF_CGROUP_INET4_CONNECT]      = "connect4",
> -       [BPF_CGROUP_INET6_CONNECT]      = "connect6",
> -       [BPF_CGROUP_INET4_POST_BIND]    = "post_bind4",
> -       [BPF_CGROUP_INET6_POST_BIND]    = "post_bind6",
> -       [BPF_CGROUP_INET4_GETPEERNAME]  = "getpeername4",
> -       [BPF_CGROUP_INET6_GETPEERNAME]  = "getpeername6",
> -       [BPF_CGROUP_INET4_GETSOCKNAME]  = "getsockname4",
> -       [BPF_CGROUP_INET6_GETSOCKNAME]  = "getsockname6",
> -       [BPF_CGROUP_UDP4_SENDMSG]       = "sendmsg4",
> -       [BPF_CGROUP_UDP6_SENDMSG]       = "sendmsg6",
> -       [BPF_CGROUP_SYSCTL]             = "sysctl",
> -       [BPF_CGROUP_UDP4_RECVMSG]       = "recvmsg4",
> -       [BPF_CGROUP_UDP6_RECVMSG]       = "recvmsg6",
> -       [BPF_CGROUP_GETSOCKOPT]         = "getsockopt",
> -       [BPF_CGROUP_SETSOCKOPT]         = "setsockopt",
> +       [BPF_CGROUP_INET_INGRESS]               = "ingress",
> +       [BPF_CGROUP_INET_EGRESS]                = "egress",
> +       [BPF_CGROUP_INET_SOCK_CREATE]           = "sock_create",
> +       [BPF_CGROUP_INET_SOCK_RELEASE]          = "sock_release",
> +       [BPF_CGROUP_SOCK_OPS]                   = "sock_ops",
> +       [BPF_CGROUP_DEVICE]                     = "device",
> +       [BPF_CGROUP_INET4_BIND]                 = "bind4",
> +       [BPF_CGROUP_INET6_BIND]                 = "bind6",
> +       [BPF_CGROUP_INET4_CONNECT]              = "connect4",
> +       [BPF_CGROUP_INET6_CONNECT]              = "connect6",
> +       [BPF_CGROUP_INET4_POST_BIND]            = "post_bind4",
> +       [BPF_CGROUP_INET6_POST_BIND]            = "post_bind6",
> +       [BPF_CGROUP_INET4_GETPEERNAME]          = "getpeername4",
> +       [BPF_CGROUP_INET6_GETPEERNAME]          = "getpeername6",
> +       [BPF_CGROUP_INET4_GETSOCKNAME]          = "getsockname4",
> +       [BPF_CGROUP_INET6_GETSOCKNAME]          = "getsockname6",
> +       [BPF_CGROUP_UDP4_SENDMSG]               = "sendmsg4",
> +       [BPF_CGROUP_UDP6_SENDMSG]               = "sendmsg6",
> +       [BPF_CGROUP_SYSCTL]                     = "sysctl",
> +       [BPF_CGROUP_UDP4_RECVMSG]               = "recvmsg4",
> +       [BPF_CGROUP_UDP6_RECVMSG]               = "recvmsg6",
> +       [BPF_CGROUP_GETSOCKOPT]                 = "getsockopt",
> +       [BPF_CGROUP_SETSOCKOPT]                 = "setsockopt",
>
> -       [BPF_SK_SKB_STREAM_PARSER]      = "sk_skb_stream_parser",
> -       [BPF_SK_SKB_STREAM_VERDICT]     = "sk_skb_stream_verdict",
> -       [BPF_SK_SKB_VERDICT]            = "sk_skb_verdict",
> -       [BPF_SK_MSG_VERDICT]            = "sk_msg_verdict",
> -       [BPF_LIRC_MODE2]                = "lirc_mode2",
> -       [BPF_FLOW_DISSECTOR]            = "flow_dissector",
> -       [BPF_TRACE_RAW_TP]              = "raw_tp",
> -       [BPF_TRACE_FENTRY]              = "fentry",
> -       [BPF_TRACE_FEXIT]               = "fexit",
> -       [BPF_MODIFY_RETURN]             = "mod_ret",
> -       [BPF_LSM_MAC]                   = "lsm_mac",
> -       [BPF_SK_LOOKUP]                 = "sk_lookup",
> +       [BPF_SK_SKB_STREAM_PARSER]              = "sk_skb_stream_parser",
> +       [BPF_SK_SKB_STREAM_VERDICT]             = "sk_skb_stream_verdict",
> +       [BPF_SK_SKB_VERDICT]                    = "sk_skb_verdict",
> +       [BPF_SK_MSG_VERDICT]                    = "sk_msg_verdict",
> +       [BPF_LIRC_MODE2]                        = "lirc_mode2",
> +       [BPF_FLOW_DISSECTOR]                    = "flow_dissector",
> +       [BPF_TRACE_RAW_TP]                      = "raw_tp",
> +       [BPF_TRACE_FENTRY]                      = "fentry",
> +       [BPF_TRACE_FEXIT]                       = "fexit",
> +       [BPF_MODIFY_RETURN]                     = "mod_ret",
> +       [BPF_LSM_MAC]                           = "lsm_mac",
> +       [BPF_SK_LOOKUP]                         = "sk_lookup",
> +       [BPF_TRACE_ITER]                        = "trace_iter",
> +       [BPF_XDP_DEVMAP]                        = "xdp_devmap",
> +       [BPF_XDP_CPUMAP]                        = "xdp_cpumap",
> +       [BPF_XDP]                               = "xdp",
> +       [BPF_SK_REUSEPORT_SELECT]               = "sk_skb_reuseport_select",
> +       [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE]    = "sk_skb_reuseport_select_or_migrate",
>  };
>

you are ruining Git blaming abilities for purely aesthetic reasons,
which are not good enough reasons, IMO. Please don't do this, this
nice alignment is nice, but definitely not necessary. So whatever is
longer then the "default indentation", just add another tab or two and
be done with it. That way we can actually see what was added in this
patch, btw.

>  void p_err(const char *fmt, ...)
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index cc48726740ad..1ee87225543b 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -2245,8 +2245,8 @@ static int do_help(int argc, char **argv)
>                 "                 cgroup/sendmsg6 | cgroup/recvmsg4 | cgroup/recvmsg6 |\n"
>                 "                 cgroup/getsockopt | cgroup/setsockopt | cgroup/sock_release |\n"
>                 "                 struct_ops | fentry | fexit | freplace | sk_lookup }\n"
> -               "       ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
> -               "                        flow_dissector }\n"
> +               "       ATTACH_TYPE := { msg_verdict | skb_verdict | stream_verdict |\n"
> +               "                        stream_parser | flow_dissector }\n"
>                 "       METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
>                 "       " HELP_SPEC_OPTIONS "\n"
>                 "",
> --
> 2.30.2
>

  reply	other threads:[~2021-07-30 18:52 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 16:29 [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates Quentin Monnet
2021-07-30 18:45   ` Andrii Nakryiko
2021-07-30 21:46     ` Quentin Monnet
2021-07-30 21:54       ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 2/7] selftests/bpf: check consistency between bpftool source, doc, completion Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types Quentin Monnet
2021-07-30 18:52   ` Andrii Nakryiko [this message]
2021-07-30 21:47     ` Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 4/7] tools: bpftool: update and synchronise option list in doc and help msg Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 5/7] selftests/bpf: update bpftool's consistency script for checking options Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options Quentin Monnet
2021-07-30 18:59   ` Andrii Nakryiko
2021-07-30 21:48     ` Quentin Monnet
2021-07-30 22:10       ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 7/7] tools: bpftool: complete metrics list in "bpftool prog profile" doc Quentin Monnet
2021-07-30 19:06 ` [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Andrii Nakryiko
2021-07-30 21:48   ` Quentin Monnet
2021-07-30 21:58     ` Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEf4BzYp2QgaL9ORzs0sWk6KO63Q-9ixU-vOsFfLckE3-bPg6A@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).