[ovs-dev] [PATCH 07/11] datapath: Load and reference the NAT helper.
Yifeng Sun
pkusunyifeng at gmail.com
Mon Oct 14 23:35:55 UTC 2019
LGTM, thanks.
Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
On Mon, Oct 14, 2019 at 10:54 AM Yi-Hung Wei <yihung.wei at gmail.com> wrote:
>
> This commit backports the following upstream commit, and two functions
> in nf_conntrack_helper.h.
>
> Upstream commit:
> commit fec9c271b8f1bde1086be5aa415cdb586e0dc800
> Author: Flavio Leitner <fbl at 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 at redhat.com>
> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
>
> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
> ---
> 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list