Re: [PATCH v6 1/3] netfilter: x_tables: Merge xt_*.h and ipt_*.h files which has same name.

[Jozsef Kadlecsik]

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.

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 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.

Best regards,
Jozsef