On Mon, 6 Jan 2025, Szőke Benjamin wrote:
2025. 01. 06. 9:19 keltezéssel, Jozsef Kadlecsik írta:
On Mon, 6 Jan 2025, egyszeregy@xxxxxxxxxxx wrote:
From: Benjamin Szőke <egyszeregy@xxxxxxxxxxx>
Merge xt_*.h, ipt_*.h and ip6t_*.h header files, which has
same upper and lower case name format.
Add #pragma message about recommended to use
header files with lower case format in the future.
Signed-off-by: Benjamin Szőke <egyszeregy@xxxxxxxxxxx>
---
include/uapi/linux/netfilter/xt_CONNMARK.h | 8 +++---
include/uapi/linux/netfilter/xt_DSCP.h | 22 ++--------------
include/uapi/linux/netfilter/xt_MARK.h | 8 +++---
include/uapi/linux/netfilter/xt_RATEEST.h | 12 ++-------
include/uapi/linux/netfilter/xt_TCPMSS.h | 14 ++++------
include/uapi/linux/netfilter/xt_connmark.h | 7 +++--
include/uapi/linux/netfilter/xt_dscp.h | 20 +++++++++++---
include/uapi/linux/netfilter/xt_mark.h | 6 ++---
include/uapi/linux/netfilter/xt_rateest.h | 15 ++++++++---
include/uapi/linux/netfilter/xt_tcpmss.h | 12 ++++++---
include/uapi/linux/netfilter_ipv4/ipt_ECN.h | 29 ++-------------------
include/uapi/linux/netfilter_ipv4/ipt_TTL.h | 25 ++++--------------
include/uapi/linux/netfilter_ipv4/ipt_ecn.h | 26 ++++++++++++++++++
include/uapi/linux/netfilter_ipv4/ipt_ttl.h | 23 +++++++++++++---
include/uapi/linux/netfilter_ipv6/ip6t_HL.h | 26 ++++--------------
include/uapi/linux/netfilter_ipv6/ip6t_hl.h | 22 +++++++++++++---
net/ipv4/netfilter/ipt_ECN.c | 2 +-
net/netfilter/xt_DSCP.c | 2 +-
net/netfilter/xt_HL.c | 4 +--
net/netfilter/xt_RATEEST.c | 2 +-
net/netfilter/xt_TCPMSS.c | 2 +-
21 files changed, 143 insertions(+), 144 deletions(-)
Technically you split up your single patch into multiple parts but not
separated it into functionally disjunct parts. So please prepare
- one patch for
include/uapi/linux/netfilter_ipv6/ip6t_HL.h
include/uapi/linux/netfilter_ipv6/ip6t_hl.h
net/netfilter/xt_HL.c
net/netfilter/xt_hl.c
[ I'd prefer corresponding Kconfig and Makefile changes as well]
- one patch for
include/uapi/linux/netfilter/xt_RATEEST.h
include/uapi/linux/netfilter/xt_rateest.h
net/netfilter/xt_RATEEST.c
net/netfilter/xt_rateest.c
[I'd prefer corresponding Kconfig and Makefile changes as well]
- and so on...
That way the reviewers can follow what was moved from where to where in a
functionally compact way.
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".
It'd mean 8 patches according to the merged match/TARGET files: mark/MARK,
connmark/CONNMARK, dscp/DSCP, rateest/RATEEST, tcpmss/TCPMSS, ecn/ECN,
ttl/TTL, hl/HL. Each one of them would be a unit which then could be
reviewed, tested independently all of the other ones.
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
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.
https://github.com/torvalds/linux/commit/1ee2f4757ff025b74569cce922147a6a8734b670
Thanks the suggestion: still, all changes are lumped together and cannot
be handled separatedly.
Also, mechanically moving the comments results in text like this:
/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-/* ip6tables module for matching the Hop Limit value
+/* Hop Limit modification module for ip6tables
+ * ip6tables module for matching the Hop Limit value
which is ... not too nice. The comments need manual fixing.
I do not know what small and compact "title" should be good here in the
merged header files. Most simplest solution was to copy paste them and merge
these titles text.
It's pretty trivial in the example: "ip6tables module for
matching/modifying the Hop Limit value". But any automated merging needs
manual verifying and fixing if needed.
You should know it better, please send a new compact and perfectly good
"title" text for all header files which are in the patchset and i can change
them finally. I think it is out of my scope in this business.
Sorry, but no: it's your responsibility to produce proper patches,
including the modified comments.
I also still don't like adding pragmas to emit warnings about
deprecated header files. It doesn't make breaking API easier and it
doesn't make possible to remove the warnings and enforce the changes
just after a few kernel releases.
I also still like adding pragmas, because duplicating these header files
is not acceptable in SW dev/coding. It must have to be taught for the
user how should use it in the future. This is a common way in any SW,
for example Python or Matlab always send a notice in run-time for you
which will be a deprecated things soon, when you import or start to use
an old function or module.
Why don't you think it can not help breaking API easier? This is the
bare minimum what you can do for it. Tell to user what should use
instead, then 3-5 years later you can change it finally, when 90-95%
percent of your customers learnt to it and already started to use it in
their userspace codes.
However as far as I'm concerned, breaking API is not a decided and
accepted thing. Breaking API in the kernel is not a "normal business" at
all.
Best regards,
Jozsef
