[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