[bpf-next,v7,0/3] Add TC-BPF API

Message

Kumar Kartikeya Dwivedi May 12, 2021, 10:34 a.m. UTC

This is the seventh version of the TC-BPF series.

It adds a simple API that uses netlink to attach the tc filter and its bpf
classifier program. Currently, a user needs to shell out to the tc command line
to be able to create filters and attach SCHED_CLS programs as classifiers. With
the help of this API, it will be possible to use libbpf for doing all parts of
bpf program setup and attach.

Changelog contains details of patchset evolution.

In an effort to keep discussion focused, this series doesn't have the high level
TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
hence that will be submitted as a separate patchset based on this.

The individual commit messages contain more details, and also a brief summary of
the API.

Changelog:
----------
v6 -> v7
v6: https://lore.kernel.org/bpf/[e-mail]

 * Address all comments from Daniel
   * Rename BPF_NL_* to NL_*
   * Make bpf_tc_query only support targeted query
   * Adjust inconsistencies in the commit message
   * Drop RTM_GETTFILTER NLM_F_DUMP support
   * Other misc cleanups (also remove bpf_tc_query selftest for dump mode)

v5 -> v6
v5: https://lore.kernel.org/bpf/[e-mail]

 * Address all comments from Andrii.
 * Reorganize selftest to make logical separation between each test's set up
   and clean up more clear to the reader. Also add a common way to test
   different combination of opts.
 * Cleanup the commit message a bit.
 * Fix instances of ret < 0 && ret == -ENOENT pattern everywhere.
 * Use C89 declaration syntax.
 * Drop PRIu32.
 * Move flags to bpf_tc_opts and bpf_tc_hook.
 * Other misc comments.

v4 -> v5
v4: https://lore.kernel.org/bpf/[e-mail]

 * Added bpf_tc_hook to represent the attach location of a filter.
 * Removed the bpf_tc_ctx context object, refactored code to not assume shared
   open socket across operations on the same ctx.
 * Add a helper libbpf_nl_send_recv that wraps socket creation, sending and
   receiving the netlink message.
 * Extended netlink code to cut short message processing using BPF_NL_DONE. This
   is used in a few places to return early to the user and discard remaining
   data.
 * selftests rewrite and expansion, considering API is looking more solid now.
 * Documented the API assumptions and behaviour in the commit that adds it,
   along with a few basic usage examples.
 * Dropped documentation from libbpf.h.
 * Relax some restrictions on bpf_tc_query to make it more useful (e.g. to
   detect if any filters exist).
 * Incorporate other minor suggestions from previous review (Andrii and Daniel).

v3 -> v4
v3: https://lore.kernel.org/bpf/[e-mail]

 * Added a concept of bpf_tc_ctx context structure representing the attach point.
   The qdisc setup and delete is tied to this object's lifetime if it succeeds
   in creating the clsact qdisc when the attach point is BPF_TC_INGRESS or
   BPF_TC_EGRESS. Qdisc is only deleted when there are no filters attached to
   it.
 * Refactored all API functions to take ctx.
 * Removed bpf_tc_info, bpf_tc_attach_id, instead reused bpf_tc_opts for filling
   in attributes in various API functions (including query).
 * Explicitly documented the expectation of each function regarding the opts
   fields set. Added some small notes for the defaults chosen by the API.
 * Rename bpf_tc_get_info to bpf_tc_query
 * Keep the netlink socket open in the context structure to save on open/close
   cycles for each operation.
 * Miscellaneous adjustments due to keeping the socket open.
 * Rewrote the tests, and also added tests for testing all preconditions of the
   TC-BPF API.
 * We now use bpf skeleton in examples and tests.

v2 -> v3
v2: https://lore.kernel.org/bpf/[e-mail]

 * bpf_tc_cls_* -> bpf_tc_* rename
 * bpf_tc_attach_id now only consists of handle and priority, the two variables
   that user may or may not set.
 * bpf_tc_replace has been dropped, instead a replace bool is introduced in
   bpf_tc_opts for the same purpose.
 * bpf_tc_get_info now takes attach_id for filling in filter details during
   lookup instead of requiring user to do so. This also allows us to remove the
   fd parameter, as no matching is needed as long as we have all attributes
   necessary to identify a specific filter.
 * A little bit of code simplification taking into account the change above.
 * priority and protocol are now __u16 members in user facing API structs to
   reflect actual size.
 * Patch updating pkt_cls.h header has been removed, as it is unused now.
 * protocol and chain_index options have been dropped in bpf_tc_opts,
   protocol is always set to ETH_P_ALL, while chain_index is set as 0 by
   default in the kernel. This also means removal of chain_index from
   bpf_tc_attach_id, as it is unconditionally always 0.
 * bpf_tc_cls_change has been dropped
 * selftest now uses ASSERT_* macros

v1-> v2
v1: https://lore.kernel.org/bpf/[e-mail]

 * netlink helpers have been renamed to object_action style.
 * attach_id now only contains attributes that are not explicitly set. Only
   the bare minimum info is kept in it.
 * protocol is now an optional and always set to ETH_P_ALL.
 * direct-action mode is always set.
 * skip_sw and skip_hw options have also been removed.
 * bpf_tc_cls_info struct now also returns the bpf program tag and id, as
   available in the netlink response. This came up as a requirement during
   discussion with people wanting to use this functionality.
 * support for attaching SCHED_ACT programs has been dropped, as it isn't
   useful without any support for binding loaded actions to a classifier.
 * the distinction between dev and block API has been dropped, there is now
   a single set of functions and user has to pass the special ifindex value
   to indicate operation on a shared filter block on their own.
 * The high level API returning a bpf_link is gone. This was already non-
   functional for pinning and typical ownership semantics. Instead, a separate
   patchset will be sent adding a bpf_link API for attaching SCHED_CLS progs to
   the kernel, and its corresponding libbpf API.
 * The clsact qdisc is now setup automatically in a best-effort fashion whenever
   user passes in the clsact ingress or egress parent id. This is done with
   exclusive mode, such that if an ingress or clsact qdisc is already set up,
   we skip the setup and move on with filter creation.
 * Other minor changes that came up during the course of discussion and rework.

Kumar Kartikeya Dwivedi (3):
  libbpf: add netlink helpers
  libbpf: add low level TC-BPF API
  libbpf: add selftests for TC-BPF API

 tools/lib/bpf/libbpf.h                        |  43 ++
 tools/lib/bpf/libbpf.map                      |   5 +
 tools/lib/bpf/netlink.c                       | 554 ++++++++++++++++--
 tools/lib/bpf/nlattr.h                        |  48 ++
 .../testing/selftests/bpf/prog_tests/tc_bpf.c | 395 +++++++++++++
 .../testing/selftests/bpf/progs/test_tc_bpf.c |  12 +
 6 files changed, 993 insertions(+), 64 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_bpf.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_tc_bpf.c

Comments

Daniel Borkmann May 17, 2021, 5:54 p.m. UTC | #1

On 5/12/21 12:34 PM, Kumar Kartikeya Dwivedi wrote:
> This is the seventh version of the TC-BPF series.
> 
> It adds a simple API that uses netlink to attach the tc filter and its bpf
> classifier program. Currently, a user needs to shell out to the tc command line
> to be able to create filters and attach SCHED_CLS programs as classifiers. With
> the help of this API, it will be possible to use libbpf for doing all parts of
> bpf program setup and attach.
> 
> Changelog contains details of patchset evolution.
> 
> In an effort to keep discussion focused, this series doesn't have the high level
> TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> hence that will be submitted as a separate patchset based on this.
> 
> The individual commit messages contain more details, and also a brief summary of
> the API.
> 
> Changelog:
> ----------
> v6 -> v7
> v6: https://lore.kernel.org/bpf/[e-mail]
> 
>   * Address all comments from Daniel
>     * Rename BPF_NL_* to NL_*
>     * Make bpf_tc_query only support targeted query
>     * Adjust inconsistencies in the commit message
>     * Drop RTM_GETTFILTER NLM_F_DUMP support
>     * Other misc cleanups (also remove bpf_tc_query selftest for dump mode)

Still had to do a major patch cleanup on the first two before pushing out, but I
think it looks nice now and I do like the simple/straight forward API for tc/BPF.

Anyway, applied, thanks!

Andrii Nakryiko June 11, 2021, 8:01 p.m. UTC | #2

On Wed, May 12, 2021 at 3:35 AM Kumar Kartikeya Dwivedi
<[e-mail]> wrote:
>
> This is the seventh version of the TC-BPF series.
>
> It adds a simple API that uses netlink to attach the tc filter and its bpf
> classifier program. Currently, a user needs to shell out to the tc command line
> to be able to create filters and attach SCHED_CLS programs as classifiers. With
> the help of this API, it will be possible to use libbpf for doing all parts of
> bpf program setup and attach.
>
> Changelog contains details of patchset evolution.
>
> In an effort to keep discussion focused, this series doesn't have the high level
> TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> hence that will be submitted as a separate patchset based on this.
>
> The individual commit messages contain more details, and also a brief summary of
> the API.
>
> Changelog:
> ----------

Hey Kartikeya,

There were few issues flagged by Coverity after I synced libbpf to
Github. A bunch of them are netlink.c-related. Could you please take a
look and see if they are false positives or something that we can
actually fix? See links to the issues below. Thanks!

  [0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901199&mergedDefectId=141815
  [1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901193&mergedDefectId=322806
  [2] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901197&mergedDefectId=322807
  [3] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901195&mergedDefectId=322808

[...]

>
> Kumar Kartikeya Dwivedi (3):
>   libbpf: add netlink helpers
>   libbpf: add low level TC-BPF API
>   libbpf: add selftests for TC-BPF API
>
>  tools/lib/bpf/libbpf.h                        |  43 ++
>  tools/lib/bpf/libbpf.map                      |   5 +
>  tools/lib/bpf/netlink.c                       | 554 ++++++++++++++++--
>  tools/lib/bpf/nlattr.h                        |  48 ++
>  .../testing/selftests/bpf/prog_tests/tc_bpf.c | 395 +++++++++++++
>  .../testing/selftests/bpf/progs/test_tc_bpf.c |  12 +
>  6 files changed, 993 insertions(+), 64 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_bpf.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_tc_bpf.c
>
> --
> 2.31.1
>

Kumar Kartikeya Dwivedi June 11, 2021, 8:46 p.m. UTC | #3

On Sat, Jun 12, 2021 at 01:31:56AM IST, Andrii Nakryiko wrote:
> On Wed, May 12, 2021 at 3:35 AM Kumar Kartikeya Dwivedi
> <[e-mail]> wrote:
> >
> > This is the seventh version of the TC-BPF series.
> >
> > It adds a simple API that uses netlink to attach the tc filter and its bpf
> > classifier program. Currently, a user needs to shell out to the tc command line
> > to be able to create filters and attach SCHED_CLS programs as classifiers. With
> > the help of this API, it will be possible to use libbpf for doing all parts of
> > bpf program setup and attach.
> >
> > Changelog contains details of patchset evolution.
> >
> > In an effort to keep discussion focused, this series doesn't have the high level
> > TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> > hence that will be submitted as a separate patchset based on this.
> >
> > The individual commit messages contain more details, and also a brief summary of
> > the API.
> >
> > Changelog:
> > ----------
>
> Hey Kartikeya,
>
> There were few issues flagged by Coverity after I synced libbpf to
> Github. A bunch of them are netlink.c-related. Could you please take a
> look and see if they are false positives or something that we can
> actually fix? See links to the issues below. Thanks!
>
>   [0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901199&mergedDefectId=141815
>   [1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901193&mergedDefectId=322806
>   [2] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901197&mergedDefectId=322807
>   [3] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901195&mergedDefectId=322808
>

Hi Andrii,

These links don't work for me (I get a timeout). Would you know why? Is there
some other link where I can look at them?

> [...]
>
> >
> > Kumar Kartikeya Dwivedi (3):
> >   libbpf: add netlink helpers
> >   libbpf: add low level TC-BPF API
> >   libbpf: add selftests for TC-BPF API
> >
> >  tools/lib/bpf/libbpf.h                        |  43 ++
> >  tools/lib/bpf/libbpf.map                      |   5 +
> >  tools/lib/bpf/netlink.c                       | 554 ++++++++++++++++--
> >  tools/lib/bpf/nlattr.h                        |  48 ++
> >  .../testing/selftests/bpf/prog_tests/tc_bpf.c | 395 +++++++++++++
> >  .../testing/selftests/bpf/progs/test_tc_bpf.c |  12 +
> >  6 files changed, 993 insertions(+), 64 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_bpf.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_tc_bpf.c
> >
> > --
> > 2.31.1
> >

--
Kartikeya

Andrii Nakryiko June 11, 2021, 8:58 p.m. UTC | #4

On Fri, Jun 11, 2021 at 1:47 PM Kumar Kartikeya Dwivedi
<[e-mail]> wrote:
>
> On Sat, Jun 12, 2021 at 01:31:56AM IST, Andrii Nakryiko wrote:
> > On Wed, May 12, 2021 at 3:35 AM Kumar Kartikeya Dwivedi
> > <[e-mail]> wrote:
> > >
> > > This is the seventh version of the TC-BPF series.
> > >
> > > It adds a simple API that uses netlink to attach the tc filter and its bpf
> > > classifier program. Currently, a user needs to shell out to the tc command line
> > > to be able to create filters and attach SCHED_CLS programs as classifiers. With
> > > the help of this API, it will be possible to use libbpf for doing all parts of
> > > bpf program setup and attach.
> > >
> > > Changelog contains details of patchset evolution.
> > >
> > > In an effort to keep discussion focused, this series doesn't have the high level
> > > TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> > > hence that will be submitted as a separate patchset based on this.
> > >
> > > The individual commit messages contain more details, and also a brief summary of
> > > the API.
> > >
> > > Changelog:
> > > ----------
> >
> > Hey Kartikeya,
> >
> > There were few issues flagged by Coverity after I synced libbpf to
> > Github. A bunch of them are netlink.c-related. Could you please take a
> > look and see if they are false positives or something that we can
> > actually fix? See links to the issues below. Thanks!
> >
> >   [0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901199&mergedDefectId=141815
> >   [1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901193&mergedDefectId=322806
> >   [2] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901197&mergedDefectId=322807
> >   [3] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901195&mergedDefectId=322808
> >
>
> Hi Andrii,
>
> These links don't work for me (I get a timeout). Would you know why? Is there
> some other link where I can look at them?
>

Sorry, I don't know any other way to share them.
https://scan3.coverity.com/reports.htm#v40547/p11903 should show all
libbpf issues. Maybe try creating an account with coverity.com?..

> > [...]
> >
> > >
> > > Kumar Kartikeya Dwivedi (3):
> > >   libbpf: add netlink helpers
> > >   libbpf: add low level TC-BPF API
> > >   libbpf: add selftests for TC-BPF API
> > >
> > >  tools/lib/bpf/libbpf.h                        |  43 ++
> > >  tools/lib/bpf/libbpf.map                      |   5 +
> > >  tools/lib/bpf/netlink.c                       | 554 ++++++++++++++++--
> > >  tools/lib/bpf/nlattr.h                        |  48 ++
> > >  .../testing/selftests/bpf/prog_tests/tc_bpf.c | 395 +++++++++++++
> > >  .../testing/selftests/bpf/progs/test_tc_bpf.c |  12 +
> > >  6 files changed, 993 insertions(+), 64 deletions(-)
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_bpf.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/test_tc_bpf.c
> > >
> > > --
> > > 2.31.1
> > >
>
> --
> Kartikeya

Kumar Kartikeya Dwivedi June 11, 2021, 9:23 p.m. UTC | #5

On Sat, Jun 12, 2021 at 02:28:09AM IST, Andrii Nakryiko wrote:
> On Fri, Jun 11, 2021 at 1:47 PM Kumar Kartikeya Dwivedi
> <[e-mail]> wrote:
> >
> > On Sat, Jun 12, 2021 at 01:31:56AM IST, Andrii Nakryiko wrote:
> > > On Wed, May 12, 2021 at 3:35 AM Kumar Kartikeya Dwivedi
> > > <[e-mail]> wrote:
> > > >
> > > > This is the seventh version of the TC-BPF series.
> > > >
> > > > It adds a simple API that uses netlink to attach the tc filter and its bpf
> > > > classifier program. Currently, a user needs to shell out to the tc command line
> > > > to be able to create filters and attach SCHED_CLS programs as classifiers. With
> > > > the help of this API, it will be possible to use libbpf for doing all parts of
> > > > bpf program setup and attach.
> > > >
> > > > Changelog contains details of patchset evolution.
> > > >
> > > > In an effort to keep discussion focused, this series doesn't have the high level
> > > > TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> > > > hence that will be submitted as a separate patchset based on this.
> > > >
> > > > The individual commit messages contain more details, and also a brief summary of
> > > > the API.
> > > >
> > > > Changelog:
> > > > ----------
> > >
> > > Hey Kartikeya,
> > >
> > > There were few issues flagged by Coverity after I synced libbpf to
> > > Github. A bunch of them are netlink.c-related. Could you please take a
> > > look and see if they are false positives or something that we can
> > > actually fix? See links to the issues below. Thanks!
> > >
> > >   [0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901199&mergedDefectId=141815
> > >   [1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901193&mergedDefectId=322806
> > >   [2] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901197&mergedDefectId=322807
> > >   [3] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901195&mergedDefectId=322808
> > >
> >
> > Hi Andrii,
> >
> > These links don't work for me (I get a timeout). Would you know why? Is there
> > some other link where I can look at them?
> >
>
> Sorry, I don't know any other way to share them.
> https://scan3.coverity.com/reports.htm#v40547/p11903 should show all
> libbpf issues. Maybe try creating an account with coverity.com?..
>

Thanks, that helped. I now see 401 Unauthorized, so I sent an approval request.

--
Kartikeya

Andrii Nakryiko June 11, 2021, 9:56 p.m. UTC | #6

On Fri, Jun 11, 2021 at 2:25 PM Kumar Kartikeya Dwivedi
<[e-mail]> wrote:
>
> On Sat, Jun 12, 2021 at 02:28:09AM IST, Andrii Nakryiko wrote:
> > On Fri, Jun 11, 2021 at 1:47 PM Kumar Kartikeya Dwivedi
> > <[e-mail]> wrote:
> > >
> > > On Sat, Jun 12, 2021 at 01:31:56AM IST, Andrii Nakryiko wrote:
> > > > On Wed, May 12, 2021 at 3:35 AM Kumar Kartikeya Dwivedi
> > > > <[e-mail]> wrote:
> > > > >
> > > > > This is the seventh version of the TC-BPF series.
> > > > >
> > > > > It adds a simple API that uses netlink to attach the tc filter and its bpf
> > > > > classifier program. Currently, a user needs to shell out to the tc command line
> > > > > to be able to create filters and attach SCHED_CLS programs as classifiers. With
> > > > > the help of this API, it will be possible to use libbpf for doing all parts of
> > > > > bpf program setup and attach.
> > > > >
> > > > > Changelog contains details of patchset evolution.
> > > > >
> > > > > In an effort to keep discussion focused, this series doesn't have the high level
> > > > > TC-BPF API. It was clear that there is a need for a bpf_link API in the kernel,
> > > > > hence that will be submitted as a separate patchset based on this.
> > > > >
> > > > > The individual commit messages contain more details, and also a brief summary of
> > > > > the API.
> > > > >
> > > > > Changelog:
> > > > > ----------
> > > >
> > > > Hey Kartikeya,
> > > >
> > > > There were few issues flagged by Coverity after I synced libbpf to
> > > > Github. A bunch of them are netlink.c-related. Could you please take a
> > > > look and see if they are false positives or something that we can
> > > > actually fix? See links to the issues below. Thanks!
> > > >
> > > >   [0] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901199&mergedDefectId=141815
> > > >   [1] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901193&mergedDefectId=322806
> > > >   [2] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901197&mergedDefectId=322807
> > > >   [3] https://scan3.coverity.com/reports.htm#v40547/p11903/fileInstanceId=53874109&defectInstanceId=10901195&mergedDefectId=322808
> > > >
> > >
> > > Hi Andrii,
> > >
> > > These links don't work for me (I get a timeout). Would you know why? Is there
> > > some other link where I can look at them?
> > >
> >
> > Sorry, I don't know any other way to share them.
> > https://scan3.coverity.com/reports.htm#v40547/p11903 should show all
> > libbpf issues. Maybe try creating an account with coverity.com?..
> >
>
> Thanks, that helped. I now see 401 Unauthorized, so I sent an approval request.

Approved.

>
> --
> Kartikeya