On Wed, Jul 31, 2019 at 7:08 PM Jesper Dangaard Brouer
[off-list ref] wrote:

On Wed, 31 Jul 2019 03:48:20 +0900
"Daniel T. Lee" [off-list ref] wrote:

quoted

By this commit, using `bpftool net load`, user can load XDP prog on
interface. New type of enum 'net_load_type' has been made, as stated at
cover-letter, the meaning of 'load' is, prog will be loaded on interface.

Why the keyword "load" ?
Why not "attach" (and "detach")?

For BPF there is a clear distinction between the "load" and "attach"
steps.  I know this is under subcommand "net", but to follow the
conversion of other subcommands e.g. "prog" there are both "load" and
"attach" commands.

quoted

BPF prog will be loaded through libbpf 'bpf_set_link_xdp_fd'.

Again this is a "set" operation, not a "load" operation.

From earlier at cover-letter, I thought using the same word 'load' might give
confusion since XDP program is not considered as 'bpf_attach_type' and can't
be attached with 'BPF_PROG_ATTACH'.

But, according to the feedback from you and Andrii Nakryiko, replacing
the word 'load' as 'attach' would be more clear and more consistent.

quoted

Signed-off-by: Daniel T. Lee <redacted>

[...]

quoted

 static int do_show(int argc, char **argv)
 {
      struct bpf_attach_info attach_info = {};

@@ -305,13 +405,17 @@ static int do_help(int argc, char **argv)

      fprintf(stderr,
              "Usage: %s %s { show | list } [dev <devname>]\n"
+             "       %s %s load PROG LOAD_TYPE <devname>\n"

The "PROG" here does it correspond to the 'bpftool prog' syntax?:

 PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }

Yes. By using the same 'prog_parse_fd' from 'bpftool prog',
user can 'attach' XDP prog with id, pinned file or tag.

quoted

              "       %s %s help\n"
+             "\n"
+             "       " HELP_SPEC_PROGRAM "\n"
+             "       LOAD_TYPE := { xdp | xdpgeneric | xdpdrv | xdpoffload }\n"
              "Note: Only xdp and tc attachments are supported now.\n"
              "      For progs attached to cgroups, use \"bpftool cgroup\"\n"
              "      to dump program attachments. For program types\n"
              "      sk_{filter,skb,msg,reuseport} and lwt/seg6, please\n"
              "      consult iproute2.\n",

--
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

And about the enum 'NET_LOAD_TYPE_XDP_DRIVE',
'DRIVER' looks more clear to understand.

Will change to it right away.

Thanks for the review.