[ovs-dev,v2,08/12] datapath: Load and reference the NAT helper.
diff mbox series

Message ID 1571160473-46132-9-git-send-email-yihung.wei@gmail.com
State New
Headers show
Series
  • Backport upstream conntrack related patches
Related show

Commit Message

Yi-Hung Wei Oct. 15, 2019, 5:27 p.m. UTC
This commit backports the following upstream commit, and two functions
in nf_conntrack_helper.h.

Upstream commit:
commit fec9c271b8f1bde1086be5aa415cdb586e0dc800
Author: Flavio Leitner <fbl@redhat.com>
Date:   Wed Apr 17 11:46:17 2019 -0300

    openvswitch: load and reference the NAT helper.

    This improves the original commit 17c357efe5ec ("openvswitch: load
    NAT helper") where it unconditionally tries to load the module for
    every flow using NAT, so not efficient when loading multiple flows.
    It also doesn't hold any references to the NAT module while the
    flow is active.

    This change fixes those problems. It will try to load the module
    only if it's not present. It grabs a reference to the NAT module
    and holds it while the flow is active. Finally, an error message
    shows up if either actions above fails.

    Fixes: 17c357efe5ec ("openvswitch: load NAT helper")
    Signed-off-by: Flavio Leitner <fbl@redhat.com>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 acinclude.m4                                       |  4 ++++
 datapath/conntrack.c                               | 27 +++++++++++++++++-----
 .../include/net/netfilter/nf_conntrack_helper.h    | 17 ++++++++++++++
 3 files changed, 42 insertions(+), 6 deletions(-)

Comments

0-day Robot Oct. 15, 2019, 6:22 p.m. UTC | #1
Bleep bloop.  Greetings Yi-Hung Wei, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Flavio Leitner <fbl@redhat.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Yi-Hung Wei <yihung.wei@gmail.com>
Lines checked: 137, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Patch
diff mbox series

diff --git a/acinclude.m4 b/acinclude.m4
index 0e9ff2596608..2f1d30f334cb 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -902,6 +902,10 @@  AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_helper.h],
                   [nf_conntrack_helper_put],
                   [OVS_DEFINE(HAVE_NF_CONNTRACK_HELPER_PUT)])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_helper.h],
+                  [nf_nat_helper_try_module_get])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_helper.h],
+                  [nf_nat_helper_put])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h],[[[[:space:]]]SKB_GSO_UDP[[[:space:]]]],
                   [OVS_DEFINE([HAVE_SKB_GSO_UDP])])
   OVS_GREP_IFELSE([$KSRC/include/net/dst.h],[DST_NOCACHE],
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 0c0d43bec2e5..9a7eab655142 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -1391,6 +1391,7 @@  static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
 {
 	struct nf_conntrack_helper *helper;
 	struct nf_conn_help *help;
+	int ret = 0;
 
 	helper = nf_conntrack_helper_try_module_get(name, info->family,
 						    key->ip.proto);
@@ -1405,13 +1406,22 @@  static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
 		return -ENOMEM;
 	}
 
+#ifdef CONFIG_NF_NAT_NEEDED
+	if (info->nat) {
+		ret = nf_nat_helper_try_module_get(name, info->family,
+						   key->ip.proto);
+		if (ret) {
+			nf_conntrack_helper_put(helper);
+			OVS_NLERR(log, "Failed to load \"%s\" NAT helper, error: %d",
+				  name, ret);
+			return ret;
+		}
+	}
+#endif
+
 	rcu_assign_pointer(help->helper, helper);
 	info->helper = helper;
-
-	if (info->nat)
-		request_module("ip_nat_%s", name);
-
-	return 0;
+	return ret;
 }
 
 #if IS_ENABLED(CONFIG_NF_NAT_NEEDED)
@@ -1898,8 +1908,13 @@  void ovs_ct_free_action(const struct nlattr *a)
 
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info)
 {
-	if (ct_info->helper)
+	if (ct_info->helper) {
+#ifdef CONFIG_NF_NAT_NEEDED
+		if (ct_info->nat)
+			nf_nat_helper_put(ct_info->helper);
+#endif
 		nf_conntrack_helper_put(ct_info->helper);
+	}
 	if (ct_info->ct) {
 		if (ct_info->timeout[0])
 			nf_ct_destroy_timeout(ct_info->ct);
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_helper.h b/datapath/linux/compat/include/net/netfilter/nf_conntrack_helper.h
index b6a3d0bf75b3..78f97375b66e 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_helper.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_helper.h
@@ -19,4 +19,21 @@  rpl_nf_ct_helper_ext_add(struct nf_conn *ct,
 #define nf_ct_helper_ext_add rpl_nf_ct_helper_ext_add
 #endif /* HAVE_NF_CT_HELPER_EXT_ADD_TAKES_HELPER */
 
+#ifndef HAVE_NF_NAT_HELPER_TRY_MODULE_GET
+static inline int rpl_nf_nat_helper_try_module_get(const char *name, u16 l3num,
+						   u8 protonum)
+{
+	request_module("ip_nat_%s", name);
+	return 0;
+}
+#define nf_nat_helper_try_module_get rpl_nf_nat_helper_try_module_get
+#endif /* HAVE_NF_NAT_HELPER_TRY_MODULE_GET */
+
+#ifndef HAVE_NF_NAT_HELPER_PUT
+void rpl_nf_nat_helper_put(struct nf_conntrack_helper *helper)
+{
+}
+#define nf_nat_helper_put rpl_nf_nat_helper_put
+#endif /* HAVE_NF_NAT_HELPER_PUT */
+
 #endif /* _NF_CONNTRACK_HELPER_WRAPPER_H */