From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752063AbdBATn4 (ORCPT ); Wed, 1 Feb 2017 14:43:56 -0500 Received: from mx2.suse.de ([195.135.220.15]:46339 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbdBATnz (ORCPT ); Wed, 1 Feb 2017 14:43:55 -0500 Date: Wed, 1 Feb 2017 20:43:51 +0100 (CET) From: Jiri Kosina X-X-Sender: jkosina@pobox.suse.cz To: Pablo Neira Ayuso cc: Linus Torvalds , Jozsef Kadlecsik , Florian Westphal , NetFilter , coreteam@netfilter.org, Linux Kernel Mailing List , info@jablonka.cz, eric@regit.org Subject: Re: [PATCH v2] netfilter: nf_ct_helper: warn when not applying default helper assignment In-Reply-To: <20170201162704.GA3880@salvia> Message-ID: References: <20170124012859.GA6375@salvia> <20170201162704.GA3880@salvia> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 1 Feb 2017, Pablo Neira Ayuso wrote: > > +{ > > + 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)) > > This fits in one line, so I think no need for find_auto_helper(), look: It does, but OTOH we're calling the lookup more than once, and that for me justifies extra function (with a descriptive name), so that we don't have to change multiple places in case the data structure changes in the future. But I don't have a strong preference either way, so I'll change it. > > + 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"); > > You can probably remove this older message now if you prefer the new > one you wrote, actually it's a bit redundant IMO. Fair enough; we'll be issuing the warning in cases where actual "harm" would be caused, and that should be enough. I'll trim down the CC list a little bit and submit v3 shortly. Thanks, -- Jiri Kosina SUSE Labs