2022-09-07 17:10:50

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next v5 0/6] Support direct writes to nf_conn:mark

Support direct writes to nf_conn:mark from TC and XDP prog types. This
is useful when applications want to store per-connection metadata. This
is also particularly useful for applications that run both bpf and
iptables/nftables because the latter can trivially access this metadata.

One example use case would be if a bpf prog is responsible for advanced
packet classification and iptables/nftables is later used for routing
due to pre-existing/legacy code.

Past discussion:
- v4: https://lore.kernel.org/bpf/[email protected]/
- v3: https://lore.kernel.org/bpf/[email protected]/
- v2: https://lore.kernel.org/bpf/CAP01T74Sgn354dXGiFWFryu4vg+o8b9s9La1d9zEbC4LGvH4qg@mail.gmail.com/T/
- v1: https://lore.kernel.org/bpf/[email protected]/

Changes since v4:
- Use exported function pointer + mutex to handle CONFIG_NF_CONNTRACK=m
case

Changes since v3:
- Use a mutex to protect module load/unload critical section

Changes since v2:
- Remove use of NOT_INIT for btf_struct_access write path
- Disallow nf_conn writing when nf_conntrack module not loaded
- Support writing to nf_conn___init:mark

Changes since v1:
- Add unimplemented stub for when !CONFIG_BPF_SYSCALL


Daniel Xu (6):
bpf: Remove duplicate PTR_TO_BTF_ID RO check
bpf: Add stub for btf_struct_access()
bpf: Use 0 instead of NOT_INIT for btf_struct_access() writes
bpf: Export btf_type_by_id() and bpf_log()
bpf: Add support for writing to nf_conn:mark
selftests/bpf: Add tests for writing to nf_conn:mark

include/linux/bpf.h | 9 +++
include/net/netfilter/nf_conntrack_bpf.h | 23 +++++++
kernel/bpf/btf.c | 1 +
kernel/bpf/verifier.c | 4 +-
net/core/filter.c | 54 +++++++++++++++
net/ipv4/bpf_tcp_ca.c | 2 +-
net/netfilter/nf_conntrack_bpf.c | 66 ++++++++++++++++++-
net/netfilter/nf_conntrack_core.c | 1 +
.../testing/selftests/bpf/prog_tests/bpf_nf.c | 2 +
.../testing/selftests/bpf/progs/test_bpf_nf.c | 9 ++-
.../selftests/bpf/progs/test_bpf_nf_fail.c | 14 ++++
11 files changed, 178 insertions(+), 7 deletions(-)

--
2.37.1


2022-09-10 00:52:02

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH bpf-next v5 0/6] Support direct writes to nf_conn:mark

On Wed, 7 Sept 2022 at 18:41, Daniel Xu <[email protected]> wrote:
>
> Support direct writes to nf_conn:mark from TC and XDP prog types. This
> is useful when applications want to store per-connection metadata. This
> is also particularly useful for applications that run both bpf and
> iptables/nftables because the latter can trivially access this metadata.
>
> One example use case would be if a bpf prog is responsible for advanced
> packet classification and iptables/nftables is later used for routing
> due to pre-existing/legacy code.
>

There are a couple of compile time warnings when conntrack is disabled,

../net/core/filter.c:8608:1: warning: symbol 'nf_conn_btf_access_lock'
was not declared. Should it be static?
../net/core/filter.c:8611:5: warning: symbol 'nfct_bsa' was not
declared. Should it be static?

Most likely because extern declaration is guarded by ifdefs. So just
moving those out of ifdef should work.
I guess you can send that as a follow up fix, or roll it in if you end
up respinning.

Otherwise, for the series:
Acked-by: Kumar Kartikeya Dwivedi <[email protected]>

2022-09-11 00:41:53

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH bpf-next v5 0/6] Support direct writes to nf_conn:mark

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[email protected]>:

On Wed, 7 Sep 2022 10:40:35 -0600 you wrote:
> Support direct writes to nf_conn:mark from TC and XDP prog types. This
> is useful when applications want to store per-connection metadata. This
> is also particularly useful for applications that run both bpf and
> iptables/nftables because the latter can trivially access this metadata.
>
> One example use case would be if a bpf prog is responsible for advanced
> packet classification and iptables/nftables is later used for routing
> due to pre-existing/legacy code.
>
> [...]

Here is the summary with links:
- [bpf-next,v5,1/6] bpf: Remove duplicate PTR_TO_BTF_ID RO check
https://git.kernel.org/bpf/bpf-next/c/65269888c695
- [bpf-next,v5,2/6] bpf: Add stub for btf_struct_access()
https://git.kernel.org/bpf/bpf-next/c/d4f7bdb2ed7b
- [bpf-next,v5,3/6] bpf: Use 0 instead of NOT_INIT for btf_struct_access() writes
https://git.kernel.org/bpf/bpf-next/c/896f07c07da0
- [bpf-next,v5,4/6] bpf: Export btf_type_by_id() and bpf_log()
https://git.kernel.org/bpf/bpf-next/c/84c6ac417cea
- [bpf-next,v5,5/6] bpf: Add support for writing to nf_conn:mark
https://git.kernel.org/bpf/bpf-next/c/864b656f82cc
- [bpf-next,v5,6/6] selftests/bpf: Add tests for writing to nf_conn:mark
https://git.kernel.org/bpf/bpf-next/c/e2d75e954c0a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


2022-09-11 17:31:07

by Daniel Xu

[permalink] [raw]
Subject: Re: [PATCH bpf-next v5 0/6] Support direct writes to nf_conn:mark

Hi Kumar,

On Sat, Sep 10, 2022 at 02:27:38AM +0200, Kumar Kartikeya Dwivedi wrote:
> On Wed, 7 Sept 2022 at 18:41, Daniel Xu <[email protected]> wrote:
> >
> > Support direct writes to nf_conn:mark from TC and XDP prog types. This
> > is useful when applications want to store per-connection metadata. This
> > is also particularly useful for applications that run both bpf and
> > iptables/nftables because the latter can trivially access this metadata.
> >
> > One example use case would be if a bpf prog is responsible for advanced
> > packet classification and iptables/nftables is later used for routing
> > due to pre-existing/legacy code.
> >
>
> There are a couple of compile time warnings when conntrack is disabled,
>
> ../net/core/filter.c:8608:1: warning: symbol 'nf_conn_btf_access_lock'
> was not declared. Should it be static?
> ../net/core/filter.c:8611:5: warning: symbol 'nfct_bsa' was not
> declared. Should it be static?
>
> Most likely because extern declaration is guarded by ifdefs. So just
> moving those out of ifdef should work.
> I guess you can send that as a follow up fix, or roll it in if you end
> up respinning.

Hmm, I don't see how filter.c ever #include's nf_conntrack_bpf.h. So
you'd think that the warning would always be present regardless of
CONFIG_NF_CONNTRACK setting.

FWIW I can't reproduce the warning even with CONFIG_NF_CONNTRACK=n.

Maybe the extern declarations should be in include/linux/filter.h
anyways? Might be cleaner. WDYT?

> Otherwise, for the series:
> Acked-by: Kumar Kartikeya Dwivedi <[email protected]>

Thanks!

Daniel