2023-12-04 20:57:09

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

This commit moves the contents of xfrm_interface_bpf.c into a new file,
xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
to keep all the bpf integrations in a single file.

Signed-off-by: Daniel Xu <[email protected]>
---
net/xfrm/Makefile | 7 +------
net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} | 12 ++++++++----
2 files changed, 9 insertions(+), 10 deletions(-)
rename net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} (88%)

diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile
index cd47f88921f5..29fff452280d 100644
--- a/net/xfrm/Makefile
+++ b/net/xfrm/Makefile
@@ -5,12 +5,6 @@

xfrm_interface-$(CONFIG_XFRM_INTERFACE) += xfrm_interface_core.o

-ifeq ($(CONFIG_XFRM_INTERFACE),m)
-xfrm_interface-$(CONFIG_DEBUG_INFO_BTF_MODULES) += xfrm_interface_bpf.o
-else ifeq ($(CONFIG_XFRM_INTERFACE),y)
-xfrm_interface-$(CONFIG_DEBUG_INFO_BTF) += xfrm_interface_bpf.o
-endif
-
obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \
xfrm_input.o xfrm_output.o \
xfrm_sysctl.o xfrm_replay.o xfrm_device.o
@@ -21,3 +15,4 @@ obj-$(CONFIG_XFRM_USER_COMPAT) += xfrm_compat.o
obj-$(CONFIG_XFRM_IPCOMP) += xfrm_ipcomp.o
obj-$(CONFIG_XFRM_INTERFACE) += xfrm_interface.o
obj-$(CONFIG_XFRM_ESPINTCP) += espintcp.o
+obj-$(CONFIG_DEBUG_INFO_BTF) += xfrm_bpf.o
diff --git a/net/xfrm/xfrm_interface_bpf.c b/net/xfrm/xfrm_bpf.c
similarity index 88%
rename from net/xfrm/xfrm_interface_bpf.c
rename to net/xfrm/xfrm_bpf.c
index 7d5e920141e9..3d3018b87f96 100644
--- a/net/xfrm/xfrm_interface_bpf.c
+++ b/net/xfrm/xfrm_bpf.c
@@ -1,9 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Unstable XFRM Helpers for TC-BPF hook
+/* Unstable XFRM BPF helpers.
*
- * These are called from SCHED_CLS BPF programs. Note that it is
- * allowed to break compatibility for these functions since the interface they
- * are exposed through to BPF programs is explicitly unstable.
+ * Note that it is allowed to break compatibility for these functions since the
+ * interface they are exposed through to BPF programs is explicitly unstable.
*/

#include <linux/bpf.h>
@@ -12,6 +11,9 @@
#include <net/dst_metadata.h>
#include <net/xfrm.h>

+#if IS_BUILTIN(CONFIG_XFRM_INTERFACE) || \
+ (IS_MODULE(CONFIG_XFRM_INTERFACE) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
+
/* bpf_xfrm_info - XFRM metadata information
*
* Members:
@@ -108,3 +110,5 @@ int __init register_xfrm_interface_bpf(void)
return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
&xfrm_interface_kfunc_set);
}
+
+#endif /* xfrm interface */
--
2.42.1


2023-12-05 01:59:14

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

On Mon, Dec 4, 2023 at 12:56 PM Daniel Xu <[email protected]> wrote:
>
> This commit moves the contents of xfrm_interface_bpf.c into a new file,
> xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
> to keep all the bpf integrations in a single file.
>
> Signed-off-by: Daniel Xu <[email protected]>
> ---
> net/xfrm/Makefile | 7 +------
> net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} | 12 ++++++++----
> 2 files changed, 9 insertions(+), 10 deletions(-)
> rename net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} (88%)
>
> diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile
> index cd47f88921f5..29fff452280d 100644
> --- a/net/xfrm/Makefile
> +++ b/net/xfrm/Makefile
> @@ -5,12 +5,6 @@
>
> xfrm_interface-$(CONFIG_XFRM_INTERFACE) += xfrm_interface_core.o
>
> -ifeq ($(CONFIG_XFRM_INTERFACE),m)
> -xfrm_interface-$(CONFIG_DEBUG_INFO_BTF_MODULES) += xfrm_interface_bpf.o
> -else ifeq ($(CONFIG_XFRM_INTERFACE),y)
> -xfrm_interface-$(CONFIG_DEBUG_INFO_BTF) += xfrm_interface_bpf.o
> -endif
> -
> obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \
> xfrm_input.o xfrm_output.o \
> xfrm_sysctl.o xfrm_replay.o xfrm_device.o
> @@ -21,3 +15,4 @@ obj-$(CONFIG_XFRM_USER_COMPAT) += xfrm_compat.o
> obj-$(CONFIG_XFRM_IPCOMP) += xfrm_ipcomp.o
> obj-$(CONFIG_XFRM_INTERFACE) += xfrm_interface.o
> obj-$(CONFIG_XFRM_ESPINTCP) += espintcp.o
> +obj-$(CONFIG_DEBUG_INFO_BTF) += xfrm_bpf.o
...
> +#if IS_BUILTIN(CONFIG_XFRM_INTERFACE) || \
> + (IS_MODULE(CONFIG_XFRM_INTERFACE) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> +
> /* bpf_xfrm_info - XFRM metadata information
> *
> * Members:
> @@ -108,3 +110,5 @@ int __init register_xfrm_interface_bpf(void)
> return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
> &xfrm_interface_kfunc_set);
> }
> +
> +#endif /* xfrm interface */

imo the original approach was cleaner.
#ifdefs in .c should be avoided when possible.
But I'm not going to insist.

ipsec folks please ack the first 3 patches.

2023-12-07 11:52:39

by Steffen Klassert

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

On Mon, Dec 04, 2023 at 01:56:21PM -0700, Daniel Xu wrote:
> This commit moves the contents of xfrm_interface_bpf.c into a new file,
> xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
> to keep all the bpf integrations in a single file.
>
> Signed-off-by: Daniel Xu <[email protected]>
> ---
> net/xfrm/Makefile | 7 +------
> net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} | 12 ++++++++----
> 2 files changed, 9 insertions(+), 10 deletions(-)
> rename net/xfrm/{xfrm_interface_bpf.c => xfrm_bpf.c} (88%)

Adding Eyal to Cc, he added the xfrm_interface_bpf.c file.

Acked-by: Steffen Klassert <[email protected]>

2023-12-07 21:08:34

by Eyal Birger

[permalink] [raw]
Subject: Re: [devel-ipsec] [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

Hi Daniel,

On Thu, Dec 7, 2023 at 3:52 AM Steffen Klassert via Devel
<[email protected]> wrote:
>
> On Mon, Dec 04, 2023 at 01:56:21PM -0700, Daniel Xu wrote:
> > This commit moves the contents of xfrm_interface_bpf.c into a new file,
> > xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
> > to keep all the bpf integrations in a single file.

This takes away the nice ability to reload the xfrm interface
related kfuncs when reloading the xfrm interface.

I also find it a little strange that the kfuncs would be available
when the xfrm interface isn't loaded.

So imho it makes sense that these kfuncs would be built
as part of the module and not as part of the core.

Eyal.

2023-12-08 08:35:30

by Steffen Klassert

[permalink] [raw]
Subject: Re: [devel-ipsec] [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

On Thu, Dec 07, 2023 at 01:08:08PM -0800, Eyal Birger wrote:
> Hi Daniel,
>
> On Thu, Dec 7, 2023 at 3:52 AM Steffen Klassert via Devel
> <[email protected]> wrote:
> >
> > On Mon, Dec 04, 2023 at 01:56:21PM -0700, Daniel Xu wrote:
> > > This commit moves the contents of xfrm_interface_bpf.c into a new file,
> > > xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
> > > to keep all the bpf integrations in a single file.
>
> This takes away the nice ability to reload the xfrm interface
> related kfuncs when reloading the xfrm interface.
>
> I also find it a little strange that the kfuncs would be available
> when the xfrm interface isn't loaded.
>
> So imho it makes sense that these kfuncs would be built
> as part of the module and not as part of the core.

I proposed to merge all the bpf extensions into one file.
With that I wanted to avoid to have 'many' files under
/net/xfrm that fall basically under bpf maintainance
scope. But if there are practical reasons to have these
spilted, I'm OK with that too.

2023-12-09 00:05:02

by Daniel Xu

[permalink] [raw]
Subject: Re: [devel-ipsec] [PATCH bpf-next v4 01/10] xfrm: bpf: Move xfrm_interface_bpf.c to xfrm_bpf.c

Hi Eyal,

On Thu, Dec 07, 2023 at 01:08:08PM -0800, Eyal Birger wrote:
> Hi Daniel,
>
> On Thu, Dec 7, 2023 at 3:52 AM Steffen Klassert via Devel
> <[email protected]> wrote:
> >
> > On Mon, Dec 04, 2023 at 01:56:21PM -0700, Daniel Xu wrote:
> > > This commit moves the contents of xfrm_interface_bpf.c into a new file,
> > > xfrm_bpf.c This is in preparation for adding more xfrm kfuncs. We'd like
> > > to keep all the bpf integrations in a single file.
>
> This takes away the nice ability to reload the xfrm interface
> related kfuncs when reloading the xfrm interface.
>
> I also find it a little strange that the kfuncs would be available
> when the xfrm interface isn't loaded.

I think technically since the xfrm interface module does the kfunc
registration, the kfuncs would only be available after the module is
loaded.

>
> So imho it makes sense that these kfuncs would be built
> as part of the module and not as part of the core.

But yeah, point taken. I will revert this commit for v5.

>
> Eyal.

Thanks,
Daniel