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;
}