On Fri, Aug 9, 2019 at 1:48 AM Quentin Monnet
<quentin.mon...@netronome.com> wrote:
>
> 2019-08-07 11:25 UTC+0900 ~ Daniel T. Lee <danieltim...@gmail.com>
> > Since, new sub-command 'net attach/detach' has been added for
> > attaching XDP program on interface,
> > this commit documents usage and sample output of `net attach/detach`.
> >
> > Signed-off-by: Daniel T. Lee <danieltim...@gmail.com>
> > ---
> > .../bpf/bpftool/Documentation/bpftool-net.rst | 51 +++++++++++++++++--
> > 1 file changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-net.rst
> > b/tools/bpf/bpftool/Documentation/bpftool-net.rst
> > index d8e5237a2085..4ad1a380e186 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-net.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-net.rst
> > @@ -15,17 +15,22 @@ SYNOPSIS
> > *OPTIONS* := { [{ **-j** | **--json** }] [{ **-p** | **--pretty** }] }
> >
> > *COMMANDS* :=
> > - { **show** | **list** } [ **dev** name ] | **help**
> > + { **show** | **list** | **attach** | **detach** | **help** }
> >
> > NET COMMANDS
> > ============
> >
> > -| **bpftool** **net { show | list } [ dev name ]**
> > +| **bpftool** **net { show | list }** [ **dev** *name* ]
> > +| **bpftool** **net attach** *ATTACH_TYPE* *PROG* **dev** *name* [
> > **overwrite** ]
> > +| **bpftool** **net detach** *ATTACH_TYPE* **dev** *name*
>
> Nit: Could we have "name" in capital letters (everywhere in the file),
> to make this file consistent with the formatting used for
> bpftool-prog.rst and bpftool-map.rst?
>
I'll update all "name" with capital "NAME" at next version of patch.
> > | **bpftool** **net help**
> > +|
> > +| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG*
> > }
> > +| *ATTACH_TYPE* := { **xdp** | **xdpgeneric** | **xdpdrv** |
> > **xdpoffload** }
> >
> > DESCRIPTION
> > ===========
> > - **bpftool net { show | list } [ dev name ]**
> > + **bpftool net { show | list }** [ **dev** *name* ]
> > List bpf program attachments in the kernel networking
> > subsystem.
> >
> > Currently, only device driver xdp attachments and tc
> > filter
> > @@ -47,6 +52,18 @@ DESCRIPTION
> > all bpf programs attached to non clsact qdiscs, and
> > finally all
> > bpf programs attached to root and clsact qdisc.
> >
> > + **bpftool** **net attach** *ATTACH_TYPE* *PROG* **dev** *name* [
> > **overwrite** ]
> > + Attach bpf program *PROG* to network interface *name*
> > with
> > + type specified by *ATTACH_TYPE*. Previously attached bpf
> > program
> > + can be replaced by the command used with **overwrite**
> > option.
> > + Currently, *ATTACH_TYPE* only contains XDP programs.
>
> Other nit: "ATTACH_TYPE only contains XDP programs" sounds odd to me.
> Could we maybe phrase this something like: "Currently, only XDP-related
> modes are supported for ATTACH_TYPE"?
>
> Also, could you please provide a brief description of the different
> attach types? In particular, explaining what "xdp" alone stands for
> might be useful.
>
I'll replace the phrase and add brief description about the attach types.
> Thanks,
> Quentin
>
> > +
> > + **bpftool** **net detach** *ATTACH_TYPE* **dev** *name*
> > + Detach bpf program attached to network interface *name*
> > with
> > + type specified by *ATTACH_TYPE*. To detach bpf program,
> > same
> > + *ATTACH_TYPE* previously used for attach must be
> > specified.
> > + Currently, *ATTACH_TYPE* only contains XDP programs.
Thank you for taking your time for the review.
