linux-kernel - Re: [PATCH v6 1/3] netfilter: x_tables: Merge xt_*.h and ipt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c885e386-4a7a-4139-95b6-6411aa8b6b8e@lunn.ch>
Date: Mon, 6 Jan 2025 15:15:28 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Szőke Benjamin <egyszeregy@...email.hu>
Cc: Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
	Florian Westphal <fw@...len.de>,
	Pablo Neira Ayuso <pablo@...filter.org>, lorenzo@...nel.org,
	daniel@...earbox.net, leitao@...ian.org, amiculas@...co.com,
	kadlec@...filter.org, David Miller <davem@...emloft.net>,
	dsahern@...nel.org, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org,
	netfilter-devel@...r.kernel.org, coreteam@...filter.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v6 1/3] netfilter: x_tables: Merge xt_*.h and ipt_*.h
 files which has same name.

> First suggestion was to split it 2 parts, it is done, i split in 3 parts, it
> was more then needed. Your idea will lead to split it about to 20 patch
> parts, then the next problem from you could be "there are to many small
> singel patches, please reduce it".

You are missing some meaning in what i said. I said it needed to be
split into two patchsets. A patchset is a collection of patches. I
would like to see one set of patches doing the merge, and a second set
of patches doing the case insensitive changes.

Within those patch sets, you should have lots of little patches, each
of which is simple to review, has a good commit messages, and it
obviously correct.

You are unlikely to get feedback saying the patches are too
small. There is however a limit of 15 patches in a patch set. If you
actually needed 20 patches, then you break it up into two patch sets.

> If you like to see it in a human readable format you can found the full diff
> and the separted patches also in this link:
> https://github.com/torvalds/linux/compare/master...Livius90:linux:uapi

Patches are human readable, especially when they are small, and have a
good commit message. Spend a little bit of time reading patches from
people like Russell King, Oleksij Rempel, just to pick two names at
random.

> Please start to use any modern reviewing tool in 2025 and you can solve your
> problem. In GitHub history view i can see easly what was moved from where to
> where in 1-3 mouse clicking, eg.: click to xt_DSCP.h then click to xt_dscp.h
> and you can see everything nicely. So it is ready for reviewing, please sit
> down and start work on it as a maintainer, It's your turn now.

I use gitlab for the day job. It is missing some really basic features
which i think make it unsuitable for the Linux role of "Reviewer". It
also is really slow to use and does not scale to the volume of patches
you see on netdev. With some re-engineering, it might be possible to
fix these issues, but so far, i've not seen it happen.

Part of the issues here is, Linux is short of Maintainers/Reviews,
given the number of developers. So the processes are set up to make
the Maintainers/Reviews roles more efficient, pushing as much work as
possible to developers which there are plenty off. Tools like
gitlab/github don't really make the Maintainers/Reviews roles
efficient, so don't work too well for Linux.

	Andrew