LGTM, thanks. Reviewed-by: Yifeng Sun <[email protected]>
On Mon, Oct 14, 2019 at 10:54 AM Yi-Hung Wei <[email protected]> wrote: > > This commit backports the following upstream commit, and two functions > in nf_conntrack_helper.h. > > Upstream commit: > commit fec9c271b8f1bde1086be5aa415cdb586e0dc800 > Author: Flavio Leitner <[email protected]> > 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 <[email protected]> > Signed-off-by: Pablo Neira Ayuso <[email protected]> > > Signed-off-by: Yi-Hung Wei <[email protected]> > --- > acinclude.m4 | 4 ++++ > datapath/conntrack.c | 27 > +++++++++++++++++----- > .../include/net/netfilter/nf_conntrack_helper.h | 17 ++++++++++++++ > 3 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 055f5387db19..22f92723b00d 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -904,6 +904,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 */ > -- > 2.7.4 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
