From: Jeremy Sowden <jeremy@azazel.net>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>,
Florian Westphal <fw@strlen.de>,
Netfilter Devel <netfilter-devel@vger.kernel.org>
Subject: Re: [PATCH nf-next v2 00/30] Add config option checks to netfilter headers.
Date: Sat, 7 Sep 2019 20:16:59 +0100 [thread overview]
Message-ID: <20190907191658.GA6508@azazel.net> (raw)
In-Reply-To: <20190904190535.7dslwytvpff567mt@salvia>
[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]
On 2019-09-04, at 21:05:35 +0200, Pablo Neira Ayuso wrote:
> Thanks for working on this.
Happy to help.
> Could you squash a few of these patches to get a smaller patchset?
Absolutely.
> My suggestions:
>
> * Squash 01/30, 02/30 and 03/30, call this something like: "netfilter:
> add missing include guard". Just document that the chunk for the
> flowtable is fixing up a comment.
Will do.
> * For 04/30, since this is about SPDX, I would suggest you leave this
> behind and we wait for someone to make a whole pass over the
> netfilter headers to check for missing SPDX tags? Not a deal
> breaker, you can keep it in this batch if you like.
Will drop it. This was a bit speculative: I think I've got it right,
but, as you say, this may be one to leave to someone with more
expertise.
> * Squash 05/30, 06/30 and 07/30, call this I'd suggest: "netfilter:
> fix coding style errors", document the stray semi-colons, the
> Kconfig missing indent and the trailing whitespaces.
Will do.
> * Squash 09/30, 10/30, 11/30, 12/30 and 12/30. They all refer to
> #include updates, could you squash and document these updates?
Will do.
> * 14/30, "netfilter: remove superfluous header" I'd suggest you rename
> this to "netfilter: remove nf_conntrack_icmpv6.h header".
Will do.
> * 17/30 I don't think struct nf_bridge_frag_data qualifies for the
> global netfilter.h header.
What about netfilter_bridge.h?
> * Please, squash 21/30 and 22/30.
Will do.
> * With 20/30 gets more ifdef pollution to optimize a case where kernel
> is compiled without this trackers. I would prefer you keep this
> back.
>
> * 24/30 nft_set_pktinfo_ipv6_validate() definition already
> deals with this in the right way.
>
> * 25/30 nf_conntrack_zones_common.h only makes sense if NF_CONNTRACK
> is enabled, I don't understand.
>
> * 27/30 identation is not correct, not using tabs.
>
> * 26/30 is adding more #ifdef CONFIG_NETFILTER to the netfilter.h
> header. They make sense to make this new infra to compile headers,
> but from developer perspective is confusing.
>
> * 30/30 very similar to 26/30...
As I mentioned in the cover-letter the idea behind my approach was to
config out as much code as possible: if header H is only required when
config C is enabled, then wrap it in an `#if IS_ENABLED(CONFIG_C)`.
However, you're clearly not keen, and, having had a poke around in other
headers that have been moved off the blacklist, I've come to the con-
clusion that it was the wrong way to go: we want less #ifdeffery, not
more. Will rework this part of the series.
J.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-09-07 19:17 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-02 23:06 [PATCH nf-next v2 00/30] Add config option checks to netfilter headers Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 01/30] netfilter: add include guard to nf_conntrack_h323_types.h Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 02/30] netfilter: add include guard to nf_conntrack_labels.h Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 03/30] netfilter: fix include guard comment Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 04/30] netfilter: add GPL-2.0 SPDX ID's to a couple of headers Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 05/30] netfilter: remove trailing white-space Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 06/30] netfilter: fix Kconfig formatting error Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 07/30] netfilter: remove stray semicolons Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 08/30] netfilter: remove unused function declarations Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 09/30] netfilter: remove unused includes Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 10/30] netfilter: include the right header in nf_conntrack_zones.h Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 11/30] netfilter: fix inclusions of <linux/netfilter/nf_nat.h> Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 12/30] netfilter: added missing includes Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 13/30] netfilter: inline three headers Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 14/30] netfilter: remove superfluous header Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 15/30] netfilter: move inline function to a more appropriate header Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 16/30] netfilter: move code between synproxy headers Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 17/30] netfilter: move struct definition function to a more appropriate header Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 18/30] netfilter: use consistent style when defining inline functions in nf_conntrack_ecache.h Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 19/30] netfilter: replace defined(CONFIG...) || defined(CONFIG...MODULE) with IS_ENABLED(CONFIG...) Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 20/30] netfilter: wrap union nf_conntrack_proto members in CONFIG_NF_CT_PROTO_* check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 21/30] netfilter: wrap inline synproxy function in CONFIG_NETFILTER_SYNPROXY check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 22/30] netfilter: wrap inline timeout function in CONFIG_NETFILTER_TIMEOUT check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 23/30] netfilter: wrap some nat-related conntrack code in a CONFIG_NF_NAT check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 24/30] netfilter: wrap some ipv6 tables code in a CONFIG_NF_TABLES_IPV6 check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 25/30] netfilter: wrap some conntrack code in a CONFIG_NF_CONNTRACK check Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 26/30] netfilter: add CONFIG_NETFILTER check to linux/netfilter.h Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 27/30] netfilter: add NF_TPROXY config option Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 28/30] netfilter: add IP_SET_BITMAP " Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 29/30] netfilter: add IP_SET_HASH " Jeremy Sowden
2019-09-02 23:06 ` [PATCH nf-next v2 30/30] netfilter: wrap headers in CONFIG checks Jeremy Sowden
2019-09-04 13:50 ` kbuild test robot
2019-09-04 19:05 ` [PATCH nf-next v2 00/30] Add config option checks to netfilter headers Pablo Neira Ayuso
2019-09-07 19:16 ` Jeremy Sowden [this message]
2019-09-08 18:14 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190907191658.GA6508@azazel.net \
--to=jeremy@azazel.net \
--cc=fw@strlen.de \
--cc=kadlec@netfilter.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.