Re: [PATCH v2] netfilter: nf_ct_helper: warn when not applying default helper assignment

[Joe Perches]

On Wed, 2017-01-25 at 21:43 +0100, Jiri Kosina wrote:
> Rewrite the code a little bit as suggested by Linus, so that we avoid
> spaghettiing the code even more -- namely the whole decision making
> process regarding helper selection (either automatic or not) is being
> separated, so that the whole logic can be simplified and code (condition)
> duplication reduced.
[]
> diff --git a/net/netfilter/nf_conntrack_helper.c 
> b/net/netfilter/nf_conntrack_helper.c
[]
> @@ -188,6 +188,39 @@ struct nf_conn_help *
>  }
>  EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add);
>  
> +static struct nf_conntrack_helper *find_auto_helper(struct nf_conn *ct)
> +{
> +     return __nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
> +}
> +
> +static struct nf_conntrack_helper *ct_lookup_helper(struct nf_conn *ct, 
> struct net *net)
> +{
> +     struct nf_conntrack_helper *ret;
> +
> +     if (!net->ct.sysctl_auto_assign_helper) {
> +             if (net->ct.auto_assign_helper_warned)
> +                     return NULL;
> +             if (!find_auto_helper(ct))
> +                     return NULL;
> +             pr_info("nf_conntrack: default automatic helper assignment "
> +                     "has been turned off for security reasons and CT-based "
> +                     " firewall rule not found. Use the iptables CT target "
> +                     "to attach helpers instead.\n");
> +             net->ct.auto_assign_helper_warned = 1;
> +             return NULL;
> +     }
> +
> +     ret = find_auto_helper(ct);
> +     if (!ret || net->ct.auto_assign_helper_warned)
> +             return ret;
> +     pr_info("nf_conntrack: automatic helper assignment is deprecated and it 
> will "
> +             "be removed soon. Use the iptables CT target to attach helpers "
> +             " instead.\n");
> +     net->ct.auto_assign_helper_warned = 1;
> +     return ret;
> +}

There are whitespece defects concatenating these multi-line strings.

How about an exit block that emits the message like

{
        [...]
        const char *msg;

        [...]

        if (!net->ct.sysctl_auto_assign_helper) {
                if (net->ct.auto_assign_helper_warned)
                        return NULL;
                if (!find_auto_helper(ct))
                        return NULL;
                msg = "default automatic helper assignment has been turned off 
for security reasons and CT-based firewall rule not found";
                ret = NULL;
        } else {
                ret = find_auto_helper(ct);
                if (!ret || net->ct.auto_assign_helper_warned)
                        return ret;
                msg = "automatic helper assignment is deprecated and it will be 
removed soon";
                net->ct.auto_assign_helper_warned = 1;
        }

        pr_info("nf_conntrack: %s.  Use the iptables CT target to attach 
helpers instead.\n", msg);
        return ret;
}