Re: [PATCH bpf-next v4 0/8] bpf: getsockopt and setsockopt hooks

[Stanislav Fomichev <sdf@fomichev.me>]

On 06/10, Andrii Nakryiko wrote:
> On Mon, Jun 10, 2019 at 9:39 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > This series implements two new per-cgroup hooks: getsockopt and
> > setsockopt along with a new sockopt program type. The idea is pretty
> > similar to recently introduced cgroup sysctl hooks, but
> > implementation is simpler (no need to convert to/from strings).
> >
> > What this can be applied to:
> > * move business logic of what tos/priority/etc can be set by
> >   containers (either pass or reject)
> > * handle existing options (or introduce new ones) differently by
> >   propagating some information in cgroup/socket local storage
> >
> > Compared to a simple syscall/{g,s}etsockopt tracepoint, those
> > hooks are context aware. Meaning, they can access underlying socket
> > and use cgroup and socket local storage.
> 
> It's customary to include version change log for the whole patch set
> in a cover letter vs first patch. Please include it in the future.
> Thanks!
I don't think there is a precedent that strongly favors one way or
the other. If you search the mailing list, you can find both versions:
cover letter has short version log vs each patch has detailed version
log.

My reasoning for putting version log in the particular patches is to
make life of people who review the changes easier. For example, if
a particular patch doesn't have a version change log, it means that
the patch is in the same state as in the previous version and doesn't
need another round of scrutiny.

> > Stanislav Fomichev (8):
> >   bpf: implement getsockopt and setsockopt hooks
> >   bpf: sync bpf.h to tools/
> >   libbpf: support sockopt hooks
> >   selftests/bpf: test sockopt section name
> >   selftests/bpf: add sockopt test
> >   selftests/bpf: add sockopt test that exercises sk helpers
> >   bpf: add sockopt documentation
> >   bpftool: support cgroup sockopt
> >
> >  Documentation/bpf/index.rst                   |   1 +
> >  Documentation/bpf/prog_cgroup_sockopt.rst     |  39 +
> >  include/linux/bpf-cgroup.h                    |  29 +
> >  include/linux/bpf.h                           |  45 +
> >  include/linux/bpf_types.h                     |   1 +
> >  include/linux/filter.h                        |  13 +
> >  include/uapi/linux/bpf.h                      |  13 +
> >  kernel/bpf/cgroup.c                           | 262 ++++++
> >  kernel/bpf/core.c                             |   9 +
> >  kernel/bpf/syscall.c                          |  19 +
> >  kernel/bpf/verifier.c                         |  15 +
> >  net/core/filter.c                             |   2 +-
> >  net/socket.c                                  |  18 +
> >  .../bpftool/Documentation/bpftool-cgroup.rst  |   7 +-
> >  .../bpftool/Documentation/bpftool-prog.rst    |   2 +-
> >  tools/bpf/bpftool/bash-completion/bpftool     |   8 +-
> >  tools/bpf/bpftool/cgroup.c                    |   5 +-
> >  tools/bpf/bpftool/main.h                      |   1 +
> >  tools/bpf/bpftool/prog.c                      |   3 +-
> >  tools/include/uapi/linux/bpf.h                |  14 +
> >  tools/lib/bpf/libbpf.c                        |   5 +
> >  tools/lib/bpf/libbpf_probes.c                 |   1 +
> >  tools/testing/selftests/bpf/.gitignore        |   2 +
> >  tools/testing/selftests/bpf/Makefile          |   4 +-
> >  .../testing/selftests/bpf/progs/sockopt_sk.c  |  67 ++
> >  .../selftests/bpf/test_section_names.c        |  10 +
> >  tools/testing/selftests/bpf/test_sockopt.c    | 773 ++++++++++++++++++
> >  tools/testing/selftests/bpf/test_sockopt_sk.c | 156 ++++
> >  28 files changed, 1514 insertions(+), 10 deletions(-)
> >  create mode 100644 Documentation/bpf/prog_cgroup_sockopt.rst
> >  create mode 100644 tools/testing/selftests/bpf/progs/sockopt_sk.c
> >  create mode 100644 tools/testing/selftests/bpf/test_sockopt.c
> >  create mode 100644 tools/testing/selftests/bpf/test_sockopt_sk.c
> >
> > --
> > 2.22.0.rc2.383.gf4fbbf30c2-goog