linux-kernel - Re: [PATCH 05/10] netfilter: iptables: Merge ipt_ECN.h to 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: <f53d51a9-e6d6-4376-8601-420ac756b7af@freemail.hu>
Date: Tue, 7 Jan 2025 22:48:55 +0100
From: Szőke Benjamin <egyszeregy@...email.hu>
To: Jozsef Kadlecsik <kadlec@...filter.org>
Cc: fw@...len.de, pablo@...filter.org, lorenzo@...nel.org,
 daniel@...earbox.net, leitao@...ian.org, amiculas@...co.com,
 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 05/10] netfilter: iptables: Merge ipt_ECN.h to ipt_ecn.h

2025. 01. 07. 20:26 keltezéssel, Jozsef Kadlecsik írta:
> On Tue, 7 Jan 2025, egyszeregy@...email.hu wrote:
> 
>> From: Benjamin Szőke <egyszeregy@...email.hu>
>>
>> Merge ipt_ECN.h to ipt_ecn.h header file.
>>
>> Signed-off-by: Benjamin Szőke <egyszeregy@...email.hu>
>> ---
>>   include/uapi/linux/netfilter_ipv4/ipt_ECN.h | 29 +--------------------
>>   include/uapi/linux/netfilter_ipv4/ipt_ecn.h | 26 ++++++++++++++++++
>>   2 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/include/uapi/linux/netfilter_ipv4/ipt_ECN.h b/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
>> index e3630fd045b8..6727f5a44512 100644
>> --- a/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
>> +++ b/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
>> @@ -1,34 +1,7 @@
>>   /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> -/* Header file for iptables ipt_ECN target
>> - *
>> - * (C) 2002 by Harald Welte <laforge@...monks.org>
>> - *
>> - * This software is distributed under GNU GPL v2, 1991
>> - *
>> - * ipt_ECN.h,v 1.3 2002/05/29 12:17:40 laforge Exp
>> -*/
>>   #ifndef _IPT_ECN_TARGET_H
>>   #define _IPT_ECN_TARGET_H
>>   
>> -#include <linux/types.h>
>> -#include <linux/netfilter/xt_DSCP.h>
>> -
>> -#define IPT_ECN_IP_MASK	(~XT_DSCP_MASK)
> 

If it is not dropped out in the merged header file, it will cause a build error 
because of the previous bad and duplicated header architects in the UAPI:

In file included from ../net/ipv4/netfilter/ipt_ECN.c:17:
../include/uapi/linux/netfilter_ipv4/ipt_ecn.h:17:25: error: expected identifier 
before ‘(’ token
  #define IPT_ECN_IP_MASK (~XT_DSCP_MASK)
                          ^
../include/uapi/linux/netfilter_ipv4/ipt_ecn.h:27:2: note: in expansion of macro 
‘IPT_ECN_IP_MASK’
   IPT_ECN_IP_MASK       = XT_ECN_IP_MASK,
   ^~~~~~~~~~~~~~~

Please spent more time then 10 mins about the reviewing and make some test build 
and you can see there was a conflict about how mades a fixed constant in the 
code -> It is a #define vs. enum issue.

Only one style should have been used before, and not mix them.

> The definition above is removed from here but not added to ipt_ecn.h, so
> it's missing now. Please fix it in the next round of the patchset.
> 
>> -#define IPT_ECN_OP_SET_IP	0x01	/* set ECN bits of IPv4 header */
>> -#define IPT_ECN_OP_SET_ECE	0x10	/* set ECE bit of TCP header */
>> -#define IPT_ECN_OP_SET_CWR	0x20	/* set CWR bit of TCP header */
>> -
>> -#define IPT_ECN_OP_MASK		0xce
>> -
>> -struct ipt_ECN_info {
>> -	__u8 operation;	/* bitset of operations */
>> -	__u8 ip_ect;	/* ECT codepoint of IPv4 header, pre-shifted */
>> -	union {
>> -		struct {
>> -			__u8 ece:1, cwr:1; /* TCP ECT bits */
>> -		} tcp;
>> -	} proto;
>> -};
>> +#include <linux/netfilter_ipv4/ipt_ecn.h>
>>   
>>   #endif /* _IPT_ECN_TARGET_H */
>> diff --git a/include/uapi/linux/netfilter_ipv4/ipt_ecn.h b/include/uapi/linux/netfilter_ipv4/ipt_ecn.h
>> index 8121bec47026..a6d479aece21 100644
>> --- a/include/uapi/linux/netfilter_ipv4/ipt_ecn.h
>> +++ b/include/uapi/linux/netfilter_ipv4/ipt_ecn.h
>> @@ -1,10 +1,26 @@
>>   /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/* Header file for iptables ipt_ECN target and match
>> + *
>> + * (C) 2002 by Harald Welte <laforge@...monks.org>
>> + *
>> + * This software is distributed under GNU GPL v2, 1991
>> + *
>> + * ipt_ECN.h,v 1.3 2002/05/29 12:17:40 laforge Exp
>> + */
>>   #ifndef _IPT_ECN_H
>>   #define _IPT_ECN_H
>>   
>> +#include <linux/types.h>
>> +#include <linux/netfilter/xt_dscp.h>
>>   #include <linux/netfilter/xt_ecn.h>
>> +
>>   #define ipt_ecn_info xt_ecn_info
>>   
>> +#define IPT_ECN_OP_SET_IP	0x01	/* set ECN bits of IPv4 header */
>> +#define IPT_ECN_OP_SET_ECE	0x10	/* set ECE bit of TCP header */
>> +#define IPT_ECN_OP_SET_CWR	0x20	/* set CWR bit of TCP header */
>> +#define IPT_ECN_OP_MASK		0xce
>> +
>>   enum {
>>   	IPT_ECN_IP_MASK       = XT_ECN_IP_MASK,
>>   	IPT_ECN_OP_MATCH_IP   = XT_ECN_OP_MATCH_IP,
>> @@ -13,4 +29,14 @@ enum {
>>   	IPT_ECN_OP_MATCH_MASK = XT_ECN_OP_MATCH_MASK,
>>   };
>>   
>> +struct ipt_ECN_info {
>> +	__u8 operation;	/* bitset of operations */
>> +	__u8 ip_ect;	/* ECT codepoint of IPv4 header, pre-shifted */
>> +	union {
>> +		struct {
>> +			__u8 ece:1, cwr:1; /* TCP ECT bits */
>> +		} tcp;
>> +	} proto;
>> +};
>> +
>>   #endif /* IPT_ECN_H */
>> -- 
>> 2.43.5
>>
>>
> 
> Best regards,
> Jozsef