linux-kernel - Re: [PATCH v2] netfilter: uapi: Merge xt_*.h/c 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <90b238e6-65d8-4a1d-b59b-e10445e4c61c@freemail.hu>
Date: Thu, 2 Jan 2025 19:53:36 +0100
From: Szőke Benjamin <egyszeregy@...email.hu>
To: Andrew Lunn <andrew@...n.ch>
Cc: fw@...len.de, pablo@...filter.org, lorenzo@...nel.org,
 daniel@...earbox.net, leitao@...ian.org, amiculas@...co.com,
 kadlec@...filter.org, 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 v2] netfilter: uapi: Merge xt_*.h/c and ipt_*.h which has
 same name.

2025. 01. 02. 18:39 keltezéssel, Andrew Lunn írta:
> On Thu, Jan 02, 2025 at 06:21:15PM +0100, egyszeregy@...email.hu wrote:
>> From: Benjamin Szőke <egyszeregy@...email.hu>
>>
>> Merge and refactoring xt_*.h, xt_*.c and ipt_*.h files which has the same
>> name in upper and lower case format. Combining these modules should provide
>> some decent memory savings.
> 
> Numbers please. We don't normally accept optimisations without some
> form of benchmark showing there is an improvement.
>   

Some of you mentioned in a reply e-mail, that is a good benefits in merging the 
codes. I do not have test result about it and i will no provide it.

>> The goal is to fix Linux repository for case-insensitive filesystem,
>> to able to clone it and editable on any operating systems.
> 
> This needs a much stronger argument, since as i already pointed out,
> how many case-insenstive file systems are still in use? Please give
> real world examples of why this matters.
> 

All of MacOS and Windows platform are case-insensitive. So it means, who like to 
edit Linux kernel code on them, then build it in a remote SSH solution, there 
are lot of them.

>>   delete mode 100644 include/uapi/linux/netfilter/xt_CONNMARK.h
>>   delete mode 100644 include/uapi/linux/netfilter/xt_DSCP.h
>>   delete mode 100644 include/uapi/linux/netfilter/xt_MARK.h
>>   delete mode 100644 include/uapi/linux/netfilter/xt_RATEEST.h
>>   delete mode 100644 include/uapi/linux/netfilter/xt_TCPMSS.h
>>   delete mode 100644 include/uapi/linux/netfilter_ipv4/ipt_ECN.h
>>   delete mode 100644 include/uapi/linux/netfilter_ipv4/ipt_TTL.h
>>   delete mode 100644 include/uapi/linux/netfilter_ipv6/ip6t_HL.h
> 
> How did you verify that there is no user space code using these
> includes?
> 
> We take ABI very seriously. You cannot break user space code.
> 
>      Andrew

This is a minimal ABI change, which have to use lower case filenames for 
example: xt_DSCP.h -> xt_dscp.h

By the way this UAPI code part was changed 8-10 years ago last time. There are 
many ugly codes, ugly styles in headers, there are no any consístent code style 
and technical code/solution between the variouse header and soruce files (struct 
names, const values etc...).

Somebody used enum for bit mask defining, somebody else use macros for the same 
scope. It is terrible to refactoring it without any ABI breaking change. Because 
of the code quality is terrible wrong, it should be much better to take care 
about to improve it with a heavy refactoring instead of just says ABI breaking 
change is not possible. In this way, sooner or later the house of cards will 
crumble. For long term maintainability shall need a big code changes here in 
UAPI in order to provide a good and maintainable clean code. (independent from 
case-insensitive, it should be needed)

First sign of it which can not be solved without ABI issue:
I think, "linux/include/uapi/linux/netfilter_ipv4
/ipt_ecn.h" and "linux/include/uapi/linux/netfilter_ipv4
/ipt_ECN.h" can not be merged without ABI breaking because "IPT_ECN_IP_MASK" is 
implemented in both in a different technical way. (It should not have accepted 
+10 years ago in the codebase, and then our legs wouldn't be tremble now for an 
ABI change)

ipt_ecn.h: 
https://github.com/torvalds/linux/blob/master/include/uapi/linux/netfilter_ipv4/ipt_ecn.h
ipt_ECN.h: 
https://github.com/torvalds/linux/blob/master/include/uapi/linux/netfilter_ipv4/ipt_ECN.h

In my merge i needed to drop "#define IPT_ECN_IP_MASK	(~XT_DSCP_MASK)" and i 
hope it will be replaced well with the other enum definition in any code.

merged ipt_ecn.h: 
https://github.com/Livius90/linux/blob/uapi-work/include/uapi/linux/netfilter_ipv4/ipt_ecn.h

> 
> ---
> pw-bot: cr