[bpf-next,v3,0/3] Add more bpf_*_ct_lookup() selftests

Message

Daniel Xu Aug. 10, 2022, 11:16 p.m. UTC

This patchset adds more bpf_*_ct_lookup() selftests. The goal is to test
interaction with netfilter subsystem as well as reading from `struct
nf_conn`. The first is important when migrating legacy systems towards
bpf. The latter is important in general to take full advantage of
connection tracking.

I'll follow this patchset up with support for writing to `struct nf_conn`.

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

Changes since v2:
- Add bpf-ci kconfig changes

Changes since v1:
- Reword commit message / cover letter to not mention connmark writing


Daniel Xu (3):
  selftests/bpf: Add existing connection bpf_*_ct_lookup() test
  selftests/bpf: Add connmark read test
  selftests/bpf: Update CI kconfig

 tools/testing/selftests/bpf/config            |  2 +
 .../testing/selftests/bpf/prog_tests/bpf_nf.c | 60 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_bpf_nf.c | 21 +++++++
 3 files changed, 83 insertions(+)

Comments

Kumar Kartikeya Dwivedi Aug. 11, 2022, 12:25 a.m. UTC | #1

On Thu, 11 Aug 2022 at 01:16, Daniel Xu <[e-mail]> wrote:
>
> This patchset adds more bpf_*_ct_lookup() selftests. The goal is to test
> interaction with netfilter subsystem as well as reading from `struct
> nf_conn`. The first is important when migrating legacy systems towards
> bpf. The latter is important in general to take full advantage of
> connection tracking.
>

Thank you for contributing these tests. Feel free to add:
Acked-by: Kumar Kartikeya Dwivedi <[e-mail]>

People often look at selftests for usage examples these days, so it's
great to have coverage + examples for more use cases.

> I'll follow this patchset up with support for writing to `struct nf_conn`.
>

Please also cc netfilter-devel, netdev, Pablo, and Florian when you send it.

I think we can directly enable stores to ct->mark, since that is what
ctnetlink is doing too, so adding another helper for this would be
unnecessary overhead.


> Past discussion:
> - v2: https://lore.kernel.org/bpf/[e-mail]/
> - v1: https://lore.kernel.org/bpf/[e-mail]/
>
> Changes since v2:
> - Add bpf-ci kconfig changes
>
> Changes since v1:
> - Reword commit message / cover letter to not mention connmark writing
>
>
> Daniel Xu (3):
>   selftests/bpf: Add existing connection bpf_*_ct_lookup() test
>   selftests/bpf: Add connmark read test
>   selftests/bpf: Update CI kconfig
>
>  tools/testing/selftests/bpf/config            |  2 +
>  .../testing/selftests/bpf/prog_tests/bpf_nf.c | 60 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/test_bpf_nf.c | 21 +++++++
>  3 files changed, 83 insertions(+)
>
> --
> 2.37.1
>

Daniel Xu Aug. 11, 2022, 9:50 p.m. UTC | #2

Hi Kumar,

On Wed, Aug 10, 2022, at 6:25 PM, Kumar Kartikeya Dwivedi wrote:
> On Thu, 11 Aug 2022 at 01:16, Daniel Xu <[e-mail]> wrote:
>>
>> This patchset adds more bpf_*_ct_lookup() selftests. The goal is to test
>> interaction with netfilter subsystem as well as reading from `struct
>> nf_conn`. The first is important when migrating legacy systems towards
>> bpf. The latter is important in general to take full advantage of
>> connection tracking.
>>
>
> Thank you for contributing these tests. Feel free to add:
> Acked-by: Kumar Kartikeya Dwivedi <[e-mail]>
>
> People often look at selftests for usage examples these days, so it's
> great to have coverage + examples for more use cases.

I also want this interaction to still work when I start using it later :).

>
>> I'll follow this patchset up with support for writing to `struct nf_conn`.
>>
>
> Please also cc netfilter-devel, netdev, Pablo, and Florian when you send it.
>

Ack.

> I think we can directly enable stores to ct->mark, since that is what
> ctnetlink is doing too, so adding another helper for this would be
> unnecessary overhead.

Ack.

[...]

Thanks,
Danel