* [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
@ 2022-09-25 13:26 Lorenzo Bianconi
2022-09-29 8:37 ` Yauheni Kaliuta
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-09-25 13:26 UTC (permalink / raw)
To: bpf
Cc: netdev, ast, daniel, andrii, davem, kuba, edumazet, pabeni,
pablo, fw, netfilter-devel, lorenzo.bianconi, brouer, toke,
memxor, nathan
Remove circular dependency between nf_nat module and nf_conntrack one
moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
include/net/netfilter/nf_conntrack_bpf.h | 5 ++
include/net/netfilter/nf_nat.h | 14 +++++
net/netfilter/Makefile | 6 ++
net/netfilter/nf_conntrack_bpf.c | 49 ---------------
net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
net/netfilter/nf_nat_core.c | 2 +-
6 files changed, 105 insertions(+), 50 deletions(-)
create mode 100644 net/netfilter/nf_nat_bpf.c
diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
index c8b80add1142..1ce46e406062 100644
--- a/include/net/netfilter/nf_conntrack_bpf.h
+++ b/include/net/netfilter/nf_conntrack_bpf.h
@@ -4,6 +4,11 @@
#define _NF_CONNTRACK_BPF_H
#include <linux/kconfig.h>
+#include <net/netfilter/nf_conntrack.h>
+
+struct nf_conn___init {
+ struct nf_conn ct;
+};
#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
(IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index e9eb01e99d2f..cd084059a953 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
#endif
}
+#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
+ (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
+
+extern int register_nf_nat_bpf(void);
+
+#else
+
+static inline int register_nf_nat_bpf(void)
+{
+ return 0;
+}
+
+#endif
+
int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
const struct nf_hook_ops *nat_ops, unsigned int ops_count);
void nf_nat_unregister_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 06df49ea6329..0f060d100880 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -60,6 +60,12 @@ obj-$(CONFIG_NF_NAT) += nf_nat.o
nf_nat-$(CONFIG_NF_NAT_REDIRECT) += nf_nat_redirect.o
nf_nat-$(CONFIG_NF_NAT_MASQUERADE) += nf_nat_masquerade.o
+ifeq ($(CONFIG_NF_NAT),m)
+nf_nat-$(CONFIG_DEBUG_INFO_BTF_MODULES) += nf_nat_bpf.o
+else ifeq ($(CONFIG_NF_NAT),y)
+nf_nat-$(CONFIG_DEBUG_INFO_BTF) += nf_nat_bpf.o
+endif
+
# NAT helpers
obj-$(CONFIG_NF_NAT_AMANDA) += nf_nat_amanda.o
obj-$(CONFIG_NF_NAT_FTP) += nf_nat_ftp.o
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index 756ea818574e..f4ba4ff3a63b 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -14,7 +14,6 @@
#include <linux/types.h>
#include <linux/btf_ids.h>
#include <linux/net_namespace.h>
-#include <net/netfilter/nf_conntrack.h>
#include <net/netfilter/nf_conntrack_bpf.h>
#include <net/netfilter/nf_conntrack_core.h>
#include <net/netfilter/nf_nat.h>
@@ -239,10 +238,6 @@ __diag_push();
__diag_ignore_all("-Wmissing-prototypes",
"Global functions as their definitions will be in nf_conntrack BTF");
-struct nf_conn___init {
- struct nf_conn ct;
-};
-
/* bpf_xdp_ct_alloc - Allocate a new CT entry
*
* Parameters:
@@ -476,49 +471,6 @@ int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
return nf_ct_change_status_common(nfct, status);
}
-/* bpf_ct_set_nat_info - Set source or destination nat address
- *
- * Set source or destination nat address of the newly allocated
- * nf_conn before insertion. This must be invoked for referenced
- * PTR_TO_BTF_ID to nf_conn___init.
- *
- * Parameters:
- * @nfct - Pointer to referenced nf_conn object, obtained using
- * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
- * @addr - Nat source/destination address
- * @port - Nat source/destination port. Non-positive values are
- * interpreted as select a random port.
- * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
- */
-int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
- union nf_inet_addr *addr, int port,
- enum nf_nat_manip_type manip)
-{
-#if ((IS_MODULE(CONFIG_NF_NAT) && IS_MODULE(CONFIG_NF_CONNTRACK)) || \
- IS_BUILTIN(CONFIG_NF_NAT))
- struct nf_conn *ct = (struct nf_conn *)nfct;
- u16 proto = nf_ct_l3num(ct);
- struct nf_nat_range2 range;
-
- if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
- return -EINVAL;
-
- memset(&range, 0, sizeof(struct nf_nat_range2));
- range.flags = NF_NAT_RANGE_MAP_IPS;
- range.min_addr = *addr;
- range.max_addr = range.min_addr;
- if (port > 0) {
- range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
- range.min_proto.all = cpu_to_be16(port);
- range.max_proto.all = range.min_proto.all;
- }
-
- return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
-#else
- return -EOPNOTSUPP;
-#endif
-}
-
__diag_pop()
BTF_SET8_START(nf_ct_kfunc_set)
@@ -532,7 +484,6 @@ BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
BTF_SET8_END(nf_ct_kfunc_set)
static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
new file mode 100644
index 000000000000..0fa5a0bbb0ff
--- /dev/null
+++ b/net/netfilter/nf_nat_bpf.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Unstable NAT Helpers for XDP and TC-BPF hook
+ *
+ * These are called from the XDP and 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.
+ */
+
+#include <linux/bpf.h>
+#include <linux/btf_ids.h>
+#include <net/netfilter/nf_conntrack_bpf.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_nat.h>
+
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "Global functions as their definitions will be in nf_nat BTF");
+
+/* bpf_ct_set_nat_info - Set source or destination nat address
+ *
+ * Set source or destination nat address of the newly allocated
+ * nf_conn before insertion. This must be invoked for referenced
+ * PTR_TO_BTF_ID to nf_conn___init.
+ *
+ * Parameters:
+ * @nfct - Pointer to referenced nf_conn object, obtained using
+ * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
+ * @addr - Nat source/destination address
+ * @port - Nat source/destination port. Non-positive values are
+ * interpreted as select a random port.
+ * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
+ */
+int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
+ union nf_inet_addr *addr, int port,
+ enum nf_nat_manip_type manip)
+{
+ struct nf_conn *ct = (struct nf_conn *)nfct;
+ u16 proto = nf_ct_l3num(ct);
+ struct nf_nat_range2 range;
+
+ if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
+ return -EINVAL;
+
+ memset(&range, 0, sizeof(struct nf_nat_range2));
+ range.flags = NF_NAT_RANGE_MAP_IPS;
+ range.min_addr = *addr;
+ range.max_addr = range.min_addr;
+ if (port > 0) {
+ range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
+ range.min_proto.all = cpu_to_be16(port);
+ range.max_proto.all = range.min_proto.all;
+ }
+
+ return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
+}
+
+__diag_pop()
+
+BTF_SET8_START(nf_nat_kfunc_set)
+BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
+BTF_SET8_END(nf_nat_kfunc_set)
+
+static const struct btf_kfunc_id_set nf_bpf_nat_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &nf_nat_kfunc_set,
+};
+
+int register_nf_nat_bpf(void)
+{
+ int ret;
+
+ ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
+ &nf_bpf_nat_kfunc_set);
+ if (ret)
+ return ret;
+
+ return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+ &nf_bpf_nat_kfunc_set);
+}
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 7981be526f26..1ed09c9af5e5 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -1152,7 +1152,7 @@ static int __init nf_nat_init(void)
WARN_ON(nf_nat_hook != NULL);
RCU_INIT_POINTER(nf_nat_hook, &nat_hook);
- return 0;
+ return register_nf_nat_bpf();
}
static void __exit nf_nat_cleanup(void)
--
2.37.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-25 13:26 [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c Lorenzo Bianconi
@ 2022-09-29 8:37 ` Yauheni Kaliuta
2022-09-29 9:27 ` Yauheni Kaliuta
2022-09-29 9:29 ` Yauheni Kaliuta
2022-09-29 19:13 ` Martin KaFai Lau
2 siblings, 1 reply; 8+ messages in thread
From: Yauheni Kaliuta @ 2022-09-29 8:37 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: bpf, netdev, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, David S. Miller, Jakub Kicinski, Eric Dumazet,
Paolo Abeni, pablo, Florian Westphal, netfilter-devel,
Lorenzo Bianconi, Jesper Brouer, Toke Hoiland Jorgensen,
Kumar Kartikeya Dwivedi, Nathan Chancellor
Hi!
The patch leads to
depmod: ERROR: Cycle detected: nf_conntrack -> nf_nat -> nf_conntrack
when it is built as modules (due to nf_nat_setup_info() dependency)
On Sun, Sep 25, 2022 at 4:26 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> Remove circular dependency between nf_nat module and nf_conntrack one
> moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
>
> Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> include/net/netfilter/nf_nat.h | 14 +++++
> net/netfilter/Makefile | 6 ++
> net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> net/netfilter/nf_nat_core.c | 2 +-
> 6 files changed, 105 insertions(+), 50 deletions(-)
> create mode 100644 net/netfilter/nf_nat_bpf.c
>
> diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> index c8b80add1142..1ce46e406062 100644
> --- a/include/net/netfilter/nf_conntrack_bpf.h
> +++ b/include/net/netfilter/nf_conntrack_bpf.h
> @@ -4,6 +4,11 @@
> #define _NF_CONNTRACK_BPF_H
>
> #include <linux/kconfig.h>
> +#include <net/netfilter/nf_conntrack.h>
> +
> +struct nf_conn___init {
> + struct nf_conn ct;
> +};
>
> #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> index e9eb01e99d2f..cd084059a953 100644
> --- a/include/net/netfilter/nf_nat.h
> +++ b/include/net/netfilter/nf_nat.h
> @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> #endif
> }
>
> +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> +
> +extern int register_nf_nat_bpf(void);
> +
> +#else
> +
> +static inline int register_nf_nat_bpf(void)
> +{
> + return 0;
> +}
> +
> +#endif
> +
> int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
> const struct nf_hook_ops *nat_ops, unsigned int ops_count);
> void nf_nat_unregister_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 06df49ea6329..0f060d100880 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -60,6 +60,12 @@ obj-$(CONFIG_NF_NAT) += nf_nat.o
> nf_nat-$(CONFIG_NF_NAT_REDIRECT) += nf_nat_redirect.o
> nf_nat-$(CONFIG_NF_NAT_MASQUERADE) += nf_nat_masquerade.o
>
> +ifeq ($(CONFIG_NF_NAT),m)
> +nf_nat-$(CONFIG_DEBUG_INFO_BTF_MODULES) += nf_nat_bpf.o
> +else ifeq ($(CONFIG_NF_NAT),y)
> +nf_nat-$(CONFIG_DEBUG_INFO_BTF) += nf_nat_bpf.o
> +endif
> +
> # NAT helpers
> obj-$(CONFIG_NF_NAT_AMANDA) += nf_nat_amanda.o
> obj-$(CONFIG_NF_NAT_FTP) += nf_nat_ftp.o
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index 756ea818574e..f4ba4ff3a63b 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -14,7 +14,6 @@
> #include <linux/types.h>
> #include <linux/btf_ids.h>
> #include <linux/net_namespace.h>
> -#include <net/netfilter/nf_conntrack.h>
> #include <net/netfilter/nf_conntrack_bpf.h>
> #include <net/netfilter/nf_conntrack_core.h>
> #include <net/netfilter/nf_nat.h>
> @@ -239,10 +238,6 @@ __diag_push();
> __diag_ignore_all("-Wmissing-prototypes",
> "Global functions as their definitions will be in nf_conntrack BTF");
>
> -struct nf_conn___init {
> - struct nf_conn ct;
> -};
> -
> /* bpf_xdp_ct_alloc - Allocate a new CT entry
> *
> * Parameters:
> @@ -476,49 +471,6 @@ int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
> return nf_ct_change_status_common(nfct, status);
> }
>
> -/* bpf_ct_set_nat_info - Set source or destination nat address
> - *
> - * Set source or destination nat address of the newly allocated
> - * nf_conn before insertion. This must be invoked for referenced
> - * PTR_TO_BTF_ID to nf_conn___init.
> - *
> - * Parameters:
> - * @nfct - Pointer to referenced nf_conn object, obtained using
> - * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
> - * @addr - Nat source/destination address
> - * @port - Nat source/destination port. Non-positive values are
> - * interpreted as select a random port.
> - * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
> - */
> -int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
> - union nf_inet_addr *addr, int port,
> - enum nf_nat_manip_type manip)
> -{
> -#if ((IS_MODULE(CONFIG_NF_NAT) && IS_MODULE(CONFIG_NF_CONNTRACK)) || \
> - IS_BUILTIN(CONFIG_NF_NAT))
> - struct nf_conn *ct = (struct nf_conn *)nfct;
> - u16 proto = nf_ct_l3num(ct);
> - struct nf_nat_range2 range;
> -
> - if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
> - return -EINVAL;
> -
> - memset(&range, 0, sizeof(struct nf_nat_range2));
> - range.flags = NF_NAT_RANGE_MAP_IPS;
> - range.min_addr = *addr;
> - range.max_addr = range.min_addr;
> - if (port > 0) {
> - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> - range.min_proto.all = cpu_to_be16(port);
> - range.max_proto.all = range.min_proto.all;
> - }
> -
> - return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
> -#else
> - return -EOPNOTSUPP;
> -#endif
> -}
> -
> __diag_pop()
>
> BTF_SET8_START(nf_ct_kfunc_set)
> @@ -532,7 +484,6 @@ BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
> -BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> BTF_SET8_END(nf_ct_kfunc_set)
>
> static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
> diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
> new file mode 100644
> index 000000000000..0fa5a0bbb0ff
> --- /dev/null
> +++ b/net/netfilter/nf_nat_bpf.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Unstable NAT Helpers for XDP and TC-BPF hook
> + *
> + * These are called from the XDP and 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.
> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf_ids.h>
> +#include <net/netfilter/nf_conntrack_bpf.h>
> +#include <net/netfilter/nf_conntrack_core.h>
> +#include <net/netfilter/nf_nat.h>
> +
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global functions as their definitions will be in nf_nat BTF");
> +
> +/* bpf_ct_set_nat_info - Set source or destination nat address
> + *
> + * Set source or destination nat address of the newly allocated
> + * nf_conn before insertion. This must be invoked for referenced
> + * PTR_TO_BTF_ID to nf_conn___init.
> + *
> + * Parameters:
> + * @nfct - Pointer to referenced nf_conn object, obtained using
> + * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
> + * @addr - Nat source/destination address
> + * @port - Nat source/destination port. Non-positive values are
> + * interpreted as select a random port.
> + * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
> + */
> +int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
> + union nf_inet_addr *addr, int port,
> + enum nf_nat_manip_type manip)
> +{
> + struct nf_conn *ct = (struct nf_conn *)nfct;
> + u16 proto = nf_ct_l3num(ct);
> + struct nf_nat_range2 range;
> +
> + if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
> + return -EINVAL;
> +
> + memset(&range, 0, sizeof(struct nf_nat_range2));
> + range.flags = NF_NAT_RANGE_MAP_IPS;
> + range.min_addr = *addr;
> + range.max_addr = range.min_addr;
> + if (port > 0) {
> + range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> + range.min_proto.all = cpu_to_be16(port);
> + range.max_proto.all = range.min_proto.all;
> + }
> +
> + return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
> +}
> +
> +__diag_pop()
> +
> +BTF_SET8_START(nf_nat_kfunc_set)
> +BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> +BTF_SET8_END(nf_nat_kfunc_set)
> +
> +static const struct btf_kfunc_id_set nf_bpf_nat_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &nf_nat_kfunc_set,
> +};
> +
> +int register_nf_nat_bpf(void)
> +{
> + int ret;
> +
> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
> + &nf_bpf_nat_kfunc_set);
> + if (ret)
> + return ret;
> +
> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
> + &nf_bpf_nat_kfunc_set);
> +}
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 7981be526f26..1ed09c9af5e5 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -1152,7 +1152,7 @@ static int __init nf_nat_init(void)
> WARN_ON(nf_nat_hook != NULL);
> RCU_INIT_POINTER(nf_nat_hook, &nat_hook);
>
> - return 0;
> + return register_nf_nat_bpf();
> }
>
> static void __exit nf_nat_cleanup(void)
> --
> 2.37.3
>
--
WBR, Yauheni
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-29 8:37 ` Yauheni Kaliuta
@ 2022-09-29 9:27 ` Yauheni Kaliuta
0 siblings, 0 replies; 8+ messages in thread
From: Yauheni Kaliuta @ 2022-09-29 9:27 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: bpf, netdev, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, David S. Miller, Jakub Kicinski, Eric Dumazet,
Paolo Abeni, pablo, Florian Westphal, netfilter-devel,
Lorenzo Bianconi, Jesper Brouer, Toke Hoiland Jorgensen,
Kumar Kartikeya Dwivedi, Nathan Chancellor
Hi!
>>>>> On Thu, 29 Sep 2022 11:37:49 +0300, Yauheni Kaliuta wrote:
> Hi!
> The patch leads to
> depmod: ERROR: Cycle detected: nf_conntrack -> nf_nat -> nf_conntrack
> when it is built as modules (due to nf_nat_setup_info() dependency)
Oops, I replied to the actual fix :)
Sorry for the noise.
> On Sun, Sep 25, 2022 at 4:26 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>>
>> Remove circular dependency between nf_nat module and nf_conntrack one
>> moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
>>
>> Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
>> Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> Tested-by: Nathan Chancellor <nathan@kernel.org>
>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>> ---
>> include/net/netfilter/nf_conntrack_bpf.h | 5 ++
>> include/net/netfilter/nf_nat.h | 14 +++++
>> net/netfilter/Makefile | 6 ++
>> net/netfilter/nf_conntrack_bpf.c | 49 ---------------
>> net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
>> net/netfilter/nf_nat_core.c | 2 +-
>> 6 files changed, 105 insertions(+), 50 deletions(-)
>> create mode 100644 net/netfilter/nf_nat_bpf.c
>>
>> diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
>> index c8b80add1142..1ce46e406062 100644
>> --- a/include/net/netfilter/nf_conntrack_bpf.h
>> +++ b/include/net/netfilter/nf_conntrack_bpf.h
>> @@ -4,6 +4,11 @@
>> #define _NF_CONNTRACK_BPF_H
>>
>> #include <linux/kconfig.h>
>> +#include <net/netfilter/nf_conntrack.h>
>> +
>> +struct nf_conn___init {
>> + struct nf_conn ct;
>> +};
>>
>> #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
>> (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
>> diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
>> index e9eb01e99d2f..cd084059a953 100644
>> --- a/include/net/netfilter/nf_nat.h
>> +++ b/include/net/netfilter/nf_nat.h
>> @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
>> #endif
>> }
>>
>> +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
>> + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
>> +
>> +extern int register_nf_nat_bpf(void);
>> +
>> +#else
>> +
>> +static inline int register_nf_nat_bpf(void)
>> +{
>> + return 0;
>> +}
>> +
>> +#endif
>> +
>> int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
>> const struct nf_hook_ops *nat_ops, unsigned int ops_count);
>> void nf_nat_unregister_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
>> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
>> index 06df49ea6329..0f060d100880 100644
>> --- a/net/netfilter/Makefile
>> +++ b/net/netfilter/Makefile
>> @@ -60,6 +60,12 @@ obj-$(CONFIG_NF_NAT) += nf_nat.o
>> nf_nat-$(CONFIG_NF_NAT_REDIRECT) += nf_nat_redirect.o
>> nf_nat-$(CONFIG_NF_NAT_MASQUERADE) += nf_nat_masquerade.o
>>
>> +ifeq ($(CONFIG_NF_NAT),m)
>> +nf_nat-$(CONFIG_DEBUG_INFO_BTF_MODULES) += nf_nat_bpf.o
>> +else ifeq ($(CONFIG_NF_NAT),y)
>> +nf_nat-$(CONFIG_DEBUG_INFO_BTF) += nf_nat_bpf.o
>> +endif
>> +
>> # NAT helpers
>> obj-$(CONFIG_NF_NAT_AMANDA) += nf_nat_amanda.o
>> obj-$(CONFIG_NF_NAT_FTP) += nf_nat_ftp.o
>> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
>> index 756ea818574e..f4ba4ff3a63b 100644
>> --- a/net/netfilter/nf_conntrack_bpf.c
>> +++ b/net/netfilter/nf_conntrack_bpf.c
>> @@ -14,7 +14,6 @@
>> #include <linux/types.h>
>> #include <linux/btf_ids.h>
>> #include <linux/net_namespace.h>
>> -#include <net/netfilter/nf_conntrack.h>
>> #include <net/netfilter/nf_conntrack_bpf.h>
>> #include <net/netfilter/nf_conntrack_core.h>
>> #include <net/netfilter/nf_nat.h>
>> @@ -239,10 +238,6 @@ __diag_push();
>> __diag_ignore_all("-Wmissing-prototypes",
>> "Global functions as their definitions will be in nf_conntrack BTF");
>>
>> -struct nf_conn___init {
>> - struct nf_conn ct;
>> -};
>> -
>> /* bpf_xdp_ct_alloc - Allocate a new CT entry
>> *
>> * Parameters:
>> @@ -476,49 +471,6 @@ int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
>> return nf_ct_change_status_common(nfct, status);
>> }
>>
>> -/* bpf_ct_set_nat_info - Set source or destination nat address
>> - *
>> - * Set source or destination nat address of the newly allocated
>> - * nf_conn before insertion. This must be invoked for referenced
>> - * PTR_TO_BTF_ID to nf_conn___init.
>> - *
>> - * Parameters:
>> - * @nfct - Pointer to referenced nf_conn object, obtained using
>> - * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
>> - * @addr - Nat source/destination address
>> - * @port - Nat source/destination port. Non-positive values are
>> - * interpreted as select a random port.
>> - * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
>> - */
>> -int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
>> - union nf_inet_addr *addr, int port,
>> - enum nf_nat_manip_type manip)
>> -{
>> -#if ((IS_MODULE(CONFIG_NF_NAT) && IS_MODULE(CONFIG_NF_CONNTRACK)) || \
>> - IS_BUILTIN(CONFIG_NF_NAT))
>> - struct nf_conn *ct = (struct nf_conn *)nfct;
>> - u16 proto = nf_ct_l3num(ct);
>> - struct nf_nat_range2 range;
>> -
>> - if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
>> - return -EINVAL;
>> -
>> - memset(&range, 0, sizeof(struct nf_nat_range2));
>> - range.flags = NF_NAT_RANGE_MAP_IPS;
>> - range.min_addr = *addr;
>> - range.max_addr = range.min_addr;
>> - if (port > 0) {
>> - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
>> - range.min_proto.all = cpu_to_be16(port);
>> - range.max_proto.all = range.min_proto.all;
>> - }
>> -
>> - return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
>> -#else
>> - return -EOPNOTSUPP;
>> -#endif
>> -}
>> -
>> __diag_pop()
>>
>> BTF_SET8_START(nf_ct_kfunc_set)
>> @@ -532,7 +484,6 @@ BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
>> BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
>> BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
>> BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
>> -BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
>> BTF_SET8_END(nf_ct_kfunc_set)
>>
>> static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
>> diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
>> new file mode 100644
>> index 000000000000..0fa5a0bbb0ff
>> --- /dev/null
>> +++ b/net/netfilter/nf_nat_bpf.c
>> @@ -0,0 +1,79 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/* Unstable NAT Helpers for XDP and TC-BPF hook
>> + *
>> + * These are called from the XDP and 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.
>> + */
>> +
>> +#include <linux/bpf.h>
>> +#include <linux/btf_ids.h>
>> +#include <net/netfilter/nf_conntrack_bpf.h>
>> +#include <net/netfilter/nf_conntrack_core.h>
>> +#include <net/netfilter/nf_nat.h>
>> +
>> +__diag_push();
>> +__diag_ignore_all("-Wmissing-prototypes",
>> + "Global functions as their definitions will be in nf_nat BTF");
>> +
>> +/* bpf_ct_set_nat_info - Set source or destination nat address
>> + *
>> + * Set source or destination nat address of the newly allocated
>> + * nf_conn before insertion. This must be invoked for referenced
>> + * PTR_TO_BTF_ID to nf_conn___init.
>> + *
>> + * Parameters:
>> + * @nfct - Pointer to referenced nf_conn object, obtained using
>> + * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
>> + * @addr - Nat source/destination address
>> + * @port - Nat source/destination port. Non-positive values are
>> + * interpreted as select a random port.
>> + * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
>> + */
>> +int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
>> + union nf_inet_addr *addr, int port,
>> + enum nf_nat_manip_type manip)
>> +{
>> + struct nf_conn *ct = (struct nf_conn *)nfct;
>> + u16 proto = nf_ct_l3num(ct);
>> + struct nf_nat_range2 range;
>> +
>> + if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
>> + return -EINVAL;
>> +
>> + memset(&range, 0, sizeof(struct nf_nat_range2));
>> + range.flags = NF_NAT_RANGE_MAP_IPS;
>> + range.min_addr = *addr;
>> + range.max_addr = range.min_addr;
>> + if (port > 0) {
>> + range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
>> + range.min_proto.all = cpu_to_be16(port);
>> + range.max_proto.all = range.min_proto.all;
>> + }
>> +
>> + return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
>> +}
>> +
>> +__diag_pop()
>> +
>> +BTF_SET8_START(nf_nat_kfunc_set)
>> +BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
>> +BTF_SET8_END(nf_nat_kfunc_set)
>> +
>> +static const struct btf_kfunc_id_set nf_bpf_nat_kfunc_set = {
>> + .owner = THIS_MODULE,
>> + .set = &nf_nat_kfunc_set,
>> +};
>> +
>> +int register_nf_nat_bpf(void)
>> +{
>> + int ret;
>> +
>> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
>> + &nf_bpf_nat_kfunc_set);
>> + if (ret)
>> + return ret;
>> +
>> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
>> + &nf_bpf_nat_kfunc_set);
>> +}
>> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
>> index 7981be526f26..1ed09c9af5e5 100644
>> --- a/net/netfilter/nf_nat_core.c
>> +++ b/net/netfilter/nf_nat_core.c
>> @@ -1152,7 +1152,7 @@ static int __init nf_nat_init(void)
>> WARN_ON(nf_nat_hook != NULL);
>> RCU_INIT_POINTER(nf_nat_hook, &nat_hook);
>>
>> - return 0;
>> + return register_nf_nat_bpf();
>> }
>>
>> static void __exit nf_nat_cleanup(void)
>> --
>> 2.37.3
>>
> --
> WBR, Yauheni
--
WBR,
Yauheni Kaliuta
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-25 13:26 [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c Lorenzo Bianconi
2022-09-29 8:37 ` Yauheni Kaliuta
@ 2022-09-29 9:29 ` Yauheni Kaliuta
2022-09-29 19:13 ` Martin KaFai Lau
2 siblings, 0 replies; 8+ messages in thread
From: Yauheni Kaliuta @ 2022-09-29 9:29 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: bpf, netdev, ast, daniel, andrii, davem, kuba, edumazet, pabeni,
pablo, fw, netfilter-devel, lorenzo.bianconi, brouer, toke,
memxor, nathan
Hi, Lorenzo!
Tested-by: Yauheni Kaliuta <ykaliuta@redhat.com>
>>>>> On Sun, 25 Sep 2022 15:26:12 +0200, Lorenzo Bianconi wrote:
> Remove circular dependency between nf_nat module and nf_conntrack one
> moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
> Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> include/net/netfilter/nf_nat.h | 14 +++++
> net/netfilter/Makefile | 6 ++
> net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> net/netfilter/nf_nat_core.c | 2 +-
> 6 files changed, 105 insertions(+), 50 deletions(-)
> create mode 100644 net/netfilter/nf_nat_bpf.c
> diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> index c8b80add1142..1ce46e406062 100644
> --- a/include/net/netfilter/nf_conntrack_bpf.h
> +++ b/include/net/netfilter/nf_conntrack_bpf.h
> @@ -4,6 +4,11 @@
> #define _NF_CONNTRACK_BPF_H
> #include <linux/kconfig.h>
> +#include <net/netfilter/nf_conntrack.h>
> +
> +struct nf_conn___init {
> + struct nf_conn ct;
> +};
> #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> index e9eb01e99d2f..cd084059a953 100644
> --- a/include/net/netfilter/nf_nat.h
> +++ b/include/net/netfilter/nf_nat.h
> @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> #endif
> }
> +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> +
> +extern int register_nf_nat_bpf(void);
> +
> +#else
> +
> +static inline int register_nf_nat_bpf(void)
> +{
> + return 0;
> +}
> +
> +#endif
> +
> int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
> const struct nf_hook_ops *nat_ops, unsigned int ops_count);
> void nf_nat_unregister_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 06df49ea6329..0f060d100880 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -60,6 +60,12 @@ obj-$(CONFIG_NF_NAT) += nf_nat.o
> nf_nat-$(CONFIG_NF_NAT_REDIRECT) += nf_nat_redirect.o
> nf_nat-$(CONFIG_NF_NAT_MASQUERADE) += nf_nat_masquerade.o
> +ifeq ($(CONFIG_NF_NAT),m)
> +nf_nat-$(CONFIG_DEBUG_INFO_BTF_MODULES) += nf_nat_bpf.o
> +else ifeq ($(CONFIG_NF_NAT),y)
> +nf_nat-$(CONFIG_DEBUG_INFO_BTF) += nf_nat_bpf.o
> +endif
> +
> # NAT helpers
> obj-$(CONFIG_NF_NAT_AMANDA) += nf_nat_amanda.o
> obj-$(CONFIG_NF_NAT_FTP) += nf_nat_ftp.o
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index 756ea818574e..f4ba4ff3a63b 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -14,7 +14,6 @@
> #include <linux/types.h>
> #include <linux/btf_ids.h>
> #include <linux/net_namespace.h>
> -#include <net/netfilter/nf_conntrack.h>
> #include <net/netfilter/nf_conntrack_bpf.h>
> #include <net/netfilter/nf_conntrack_core.h>
> #include <net/netfilter/nf_nat.h>
> @@ -239,10 +238,6 @@ __diag_push();
> __diag_ignore_all("-Wmissing-prototypes",
> "Global functions as their definitions will be in nf_conntrack BTF");
> -struct nf_conn___init {
> - struct nf_conn ct;
> -};
> -
> /* bpf_xdp_ct_alloc - Allocate a new CT entry
> *
> * Parameters:
> @@ -476,49 +471,6 @@ int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
> return nf_ct_change_status_common(nfct, status);
> }
> -/* bpf_ct_set_nat_info - Set source or destination nat address
> - *
> - * Set source or destination nat address of the newly allocated
> - * nf_conn before insertion. This must be invoked for referenced
> - * PTR_TO_BTF_ID to nf_conn___init.
> - *
> - * Parameters:
> - * @nfct - Pointer to referenced nf_conn object, obtained using
> - * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
> - * @addr - Nat source/destination address
> - * @port - Nat source/destination port. Non-positive values are
> - * interpreted as select a random port.
> - * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
> - */
> -int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
> - union nf_inet_addr *addr, int port,
> - enum nf_nat_manip_type manip)
> -{
> -#if ((IS_MODULE(CONFIG_NF_NAT) && IS_MODULE(CONFIG_NF_CONNTRACK)) || \
> - IS_BUILTIN(CONFIG_NF_NAT))
> - struct nf_conn *ct = (struct nf_conn *)nfct;
> - u16 proto = nf_ct_l3num(ct);
> - struct nf_nat_range2 range;
> -
> - if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
> - return -EINVAL;
> -
> - memset(&range, 0, sizeof(struct nf_nat_range2));
> - range.flags = NF_NAT_RANGE_MAP_IPS;
> - range.min_addr = *addr;
> - range.max_addr = range.min_addr;
> - if (port > 0) {
> - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> - range.min_proto.all = cpu_to_be16(port);
> - range.max_proto.all = range.min_proto.all;
> - }
> -
> - return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
> -#else
> - return -EOPNOTSUPP;
> -#endif
> -}
> -
> __diag_pop()
> BTF_SET8_START(nf_ct_kfunc_set)
> @@ -532,7 +484,6 @@ BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
> -BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> BTF_SET8_END(nf_ct_kfunc_set)
> static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
> diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
> new file mode 100644
> index 000000000000..0fa5a0bbb0ff
> --- /dev/null
> +++ b/net/netfilter/nf_nat_bpf.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Unstable NAT Helpers for XDP and TC-BPF hook
> + *
> + * These are called from the XDP and 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.
> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf_ids.h>
> +#include <net/netfilter/nf_conntrack_bpf.h>
> +#include <net/netfilter/nf_conntrack_core.h>
> +#include <net/netfilter/nf_nat.h>
> +
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global functions as their definitions will be in nf_nat BTF");
> +
> +/* bpf_ct_set_nat_info - Set source or destination nat address
> + *
> + * Set source or destination nat address of the newly allocated
> + * nf_conn before insertion. This must be invoked for referenced
> + * PTR_TO_BTF_ID to nf_conn___init.
> + *
> + * Parameters:
> + * @nfct - Pointer to referenced nf_conn object, obtained using
> + * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
> + * @addr - Nat source/destination address
> + * @port - Nat source/destination port. Non-positive values are
> + * interpreted as select a random port.
> + * @manip - NF_NAT_MANIP_SRC or NF_NAT_MANIP_DST
> + */
> +int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
> + union nf_inet_addr *addr, int port,
> + enum nf_nat_manip_type manip)
> +{
> + struct nf_conn *ct = (struct nf_conn *)nfct;
> + u16 proto = nf_ct_l3num(ct);
> + struct nf_nat_range2 range;
> +
> + if (proto != NFPROTO_IPV4 && proto != NFPROTO_IPV6)
> + return -EINVAL;
> +
> + memset(&range, 0, sizeof(struct nf_nat_range2));
> + range.flags = NF_NAT_RANGE_MAP_IPS;
> + range.min_addr = *addr;
> + range.max_addr = range.min_addr;
> + if (port > 0) {
> + range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> + range.min_proto.all = cpu_to_be16(port);
> + range.max_proto.all = range.min_proto.all;
> + }
> +
> + return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
> +}
> +
> +__diag_pop()
> +
> +BTF_SET8_START(nf_nat_kfunc_set)
> +BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> +BTF_SET8_END(nf_nat_kfunc_set)
> +
> +static const struct btf_kfunc_id_set nf_bpf_nat_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &nf_nat_kfunc_set,
> +};
> +
> +int register_nf_nat_bpf(void)
> +{
> + int ret;
> +
> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
> + &nf_bpf_nat_kfunc_set);
> + if (ret)
> + return ret;
> +
> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
> + &nf_bpf_nat_kfunc_set);
> +}
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 7981be526f26..1ed09c9af5e5 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -1152,7 +1152,7 @@ static int __init nf_nat_init(void)
> WARN_ON(nf_nat_hook != NULL);
> RCU_INIT_POINTER(nf_nat_hook, &nat_hook);
> - return 0;
> + return register_nf_nat_bpf();
> }
> static void __exit nf_nat_cleanup(void)
> --
> 2.37.3
--
WBR,
Yauheni Kaliuta
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-25 13:26 [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c Lorenzo Bianconi
2022-09-29 8:37 ` Yauheni Kaliuta
2022-09-29 9:29 ` Yauheni Kaliuta
@ 2022-09-29 19:13 ` Martin KaFai Lau
2022-09-29 19:20 ` Pablo Neira Ayuso
2022-09-29 21:16 ` Lorenzo Bianconi
2 siblings, 2 replies; 8+ messages in thread
From: Martin KaFai Lau @ 2022-09-29 19:13 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: netdev, ast, daniel, andrii, davem, kuba, edumazet, pabeni,
pablo, fw, netfilter-devel, lorenzo.bianconi, brouer, toke,
memxor, nathan, bpf
On 9/25/22 6:26 AM, Lorenzo Bianconi wrote:
> Remove circular dependency between nf_nat module and nf_conntrack one
> moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
>
> Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> include/net/netfilter/nf_nat.h | 14 +++++
> net/netfilter/Makefile | 6 ++
> net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> net/netfilter/nf_nat_core.c | 2 +-
> 6 files changed, 105 insertions(+), 50 deletions(-)
> create mode 100644 net/netfilter/nf_nat_bpf.c
>
> diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> index c8b80add1142..1ce46e406062 100644
> --- a/include/net/netfilter/nf_conntrack_bpf.h
> +++ b/include/net/netfilter/nf_conntrack_bpf.h
> @@ -4,6 +4,11 @@
> #define _NF_CONNTRACK_BPF_H
>
> #include <linux/kconfig.h>
> +#include <net/netfilter/nf_conntrack.h>
> +
> +struct nf_conn___init {
> + struct nf_conn ct;
> +};
>
> #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> index e9eb01e99d2f..cd084059a953 100644
> --- a/include/net/netfilter/nf_nat.h
> +++ b/include/net/netfilter/nf_nat.h
> @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> #endif
> }
>
> +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> +
> +extern int register_nf_nat_bpf(void);
> +
> +#else
> +
> +static inline int register_nf_nat_bpf(void)
> +{
> + return 0;
> +}
> +
> +#endif
> +
This looks similar to the ones in nf_conntrack_bpf.h. Does it belong there
better? No strong opinion here.
The change looks good to me. Can someone from the netfilter team ack this piece
also?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-29 19:13 ` Martin KaFai Lau
@ 2022-09-29 19:20 ` Pablo Neira Ayuso
2022-09-29 21:16 ` Lorenzo Bianconi
2022-09-29 21:16 ` Lorenzo Bianconi
1 sibling, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2022-09-29 19:20 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Lorenzo Bianconi, netdev, ast, daniel, andrii, davem, kuba,
edumazet, pabeni, fw, netfilter-devel, lorenzo.bianconi, brouer,
toke, memxor, nathan, bpf
On Thu, Sep 29, 2022 at 12:13:45PM -0700, Martin KaFai Lau wrote:
> On 9/25/22 6:26 AM, Lorenzo Bianconi wrote:
> > Remove circular dependency between nf_nat module and nf_conntrack one
> > moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
> >
> > Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> > Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> > include/net/netfilter/nf_nat.h | 14 +++++
> > net/netfilter/Makefile | 6 ++
> > net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> > net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> > net/netfilter/nf_nat_core.c | 2 +-
> > 6 files changed, 105 insertions(+), 50 deletions(-)
> > create mode 100644 net/netfilter/nf_nat_bpf.c
> >
> > diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> > index c8b80add1142..1ce46e406062 100644
> > --- a/include/net/netfilter/nf_conntrack_bpf.h
> > +++ b/include/net/netfilter/nf_conntrack_bpf.h
> > @@ -4,6 +4,11 @@
> > #define _NF_CONNTRACK_BPF_H
> > #include <linux/kconfig.h>
> > +#include <net/netfilter/nf_conntrack.h>
> > +
> > +struct nf_conn___init {
> > + struct nf_conn ct;
> > +};
> > #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> > index e9eb01e99d2f..cd084059a953 100644
> > --- a/include/net/netfilter/nf_nat.h
> > +++ b/include/net/netfilter/nf_nat.h
> > @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> > #endif
> > }
> > +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > +
> > +extern int register_nf_nat_bpf(void);
> > +
> > +#else
> > +
> > +static inline int register_nf_nat_bpf(void)
> > +{
> > + return 0;
> > +}
> > +
> > +#endif
> > +
>
> This looks similar to the ones in nf_conntrack_bpf.h. Does it belong there
> better? No strong opinion here.
>
> The change looks good to me. Can someone from the netfilter team ack this
> piece also?
Could you move this into nf_conntrack_bpf.h ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-29 19:13 ` Martin KaFai Lau
2022-09-29 19:20 ` Pablo Neira Ayuso
@ 2022-09-29 21:16 ` Lorenzo Bianconi
1 sibling, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-09-29 21:16 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: netdev, ast, daniel, andrii, davem, kuba, edumazet, pabeni,
pablo, fw, netfilter-devel, lorenzo.bianconi, brouer, toke,
memxor, nathan, bpf
[-- Attachment #1: Type: text/plain, Size: 2579 bytes --]
> On 9/25/22 6:26 AM, Lorenzo Bianconi wrote:
> > Remove circular dependency between nf_nat module and nf_conntrack one
> > moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
> >
> > Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> > Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> > include/net/netfilter/nf_nat.h | 14 +++++
> > net/netfilter/Makefile | 6 ++
> > net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> > net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> > net/netfilter/nf_nat_core.c | 2 +-
> > 6 files changed, 105 insertions(+), 50 deletions(-)
> > create mode 100644 net/netfilter/nf_nat_bpf.c
> >
> > diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> > index c8b80add1142..1ce46e406062 100644
> > --- a/include/net/netfilter/nf_conntrack_bpf.h
> > +++ b/include/net/netfilter/nf_conntrack_bpf.h
> > @@ -4,6 +4,11 @@
> > #define _NF_CONNTRACK_BPF_H
> > #include <linux/kconfig.h>
> > +#include <net/netfilter/nf_conntrack.h>
> > +
> > +struct nf_conn___init {
> > + struct nf_conn ct;
> > +};
> > #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> > index e9eb01e99d2f..cd084059a953 100644
> > --- a/include/net/netfilter/nf_nat.h
> > +++ b/include/net/netfilter/nf_nat.h
> > @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> > #endif
> > }
> > +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > +
> > +extern int register_nf_nat_bpf(void);
> > +
> > +#else
> > +
> > +static inline int register_nf_nat_bpf(void)
> > +{
> > + return 0;
> > +}
> > +
> > +#endif
> > +
>
> This looks similar to the ones in nf_conntrack_bpf.h. Does it belong there
> better? No strong opinion here.
I have no strong opinion too. I will move it in nf_conntrack_bpf.h as
requested by Pablo.
Regards,
Lorenzo
>
> The change looks good to me. Can someone from the netfilter team ack this
> piece also?
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
2022-09-29 19:20 ` Pablo Neira Ayuso
@ 2022-09-29 21:16 ` Lorenzo Bianconi
0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-09-29 21:16 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Martin KaFai Lau, netdev, ast, daniel, andrii, davem, kuba,
edumazet, pabeni, fw, netfilter-devel, lorenzo.bianconi, brouer,
toke, memxor, nathan, bpf
[-- Attachment #1: Type: text/plain, Size: 2787 bytes --]
On Sep 29, Pablo Neira Ayuso wrote:
> On Thu, Sep 29, 2022 at 12:13:45PM -0700, Martin KaFai Lau wrote:
> > On 9/25/22 6:26 AM, Lorenzo Bianconi wrote:
> > > Remove circular dependency between nf_nat module and nf_conntrack one
> > > moving bpf_ct_set_nat_info kfunc in nf_nat_bpf.c
> > >
> > > Fixes: 0fabd2aa199f ("net: netfilter: add bpf_ct_set_nat_info kfunc helper")
> > > Suggested-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > include/net/netfilter/nf_conntrack_bpf.h | 5 ++
> > > include/net/netfilter/nf_nat.h | 14 +++++
> > > net/netfilter/Makefile | 6 ++
> > > net/netfilter/nf_conntrack_bpf.c | 49 ---------------
> > > net/netfilter/nf_nat_bpf.c | 79 ++++++++++++++++++++++++
> > > net/netfilter/nf_nat_core.c | 2 +-
> > > 6 files changed, 105 insertions(+), 50 deletions(-)
> > > create mode 100644 net/netfilter/nf_nat_bpf.c
> > >
> > > diff --git a/include/net/netfilter/nf_conntrack_bpf.h b/include/net/netfilter/nf_conntrack_bpf.h
> > > index c8b80add1142..1ce46e406062 100644
> > > --- a/include/net/netfilter/nf_conntrack_bpf.h
> > > +++ b/include/net/netfilter/nf_conntrack_bpf.h
> > > @@ -4,6 +4,11 @@
> > > #define _NF_CONNTRACK_BPF_H
> > > #include <linux/kconfig.h>
> > > +#include <net/netfilter/nf_conntrack.h>
> > > +
> > > +struct nf_conn___init {
> > > + struct nf_conn ct;
> > > +};
> > > #if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > > (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > > diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
> > > index e9eb01e99d2f..cd084059a953 100644
> > > --- a/include/net/netfilter/nf_nat.h
> > > +++ b/include/net/netfilter/nf_nat.h
> > > @@ -68,6 +68,20 @@ static inline bool nf_nat_oif_changed(unsigned int hooknum,
> > > #endif
> > > }
> > > +#if (IS_BUILTIN(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > > + (IS_MODULE(CONFIG_NF_NAT) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> > > +
> > > +extern int register_nf_nat_bpf(void);
> > > +
> > > +#else
> > > +
> > > +static inline int register_nf_nat_bpf(void)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +#endif
> > > +
> >
> > This looks similar to the ones in nf_conntrack_bpf.h. Does it belong there
> > better? No strong opinion here.
> >
> > The change looks good to me. Can someone from the netfilter team ack this
> > piece also?
>
> Could you move this into nf_conntrack_bpf.h ?
ack, I will fix it in v2.
Regards,
Lorenzo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-09-29 21:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-25 13:26 [PATCH bpf-next] net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c Lorenzo Bianconi
2022-09-29 8:37 ` Yauheni Kaliuta
2022-09-29 9:27 ` Yauheni Kaliuta
2022-09-29 9:29 ` Yauheni Kaliuta
2022-09-29 19:13 ` Martin KaFai Lau
2022-09-29 19:20 ` Pablo Neira Ayuso
2022-09-29 21:16 ` Lorenzo Bianconi
2022-09-29 21:16 ` Lorenzo Bianconi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.