| Message ID | 20250107024120.98288-2-egyszeregy@freemail.hu (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | netfilter: x_tables: Merge xt_*.h and ipt_*.h files which has same name. | expand |
On Tue, 7 Jan 2025, egyszeregy@freemail.hu wrote: > From: Benjamin Szőke <egyszeregy@freemail.hu> > > Merge xt_DSCP.h to xt_dscp.h header file. I think it'd be better worded as "Merge xt_DSCP.h into the xt_dscp.h header file." (and in the other patches as well). > Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu> > --- > include/uapi/linux/netfilter/xt_DSCP.h | 22 +--------------------- > include/uapi/linux/netfilter/xt_dscp.h | 20 ++++++++++++++++---- > 2 files changed, 17 insertions(+), 25 deletions(-) > > diff --git a/include/uapi/linux/netfilter/xt_DSCP.h b/include/uapi/linux/netfilter/xt_DSCP.h > index 223d635e8b6f..fcff72347256 100644 > --- a/include/uapi/linux/netfilter/xt_DSCP.h > +++ b/include/uapi/linux/netfilter/xt_DSCP.h > @@ -1,27 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > -/* x_tables module for setting the IPv4/IPv6 DSCP field > - * > - * (C) 2002 Harald Welte <laforge@gnumonks.org> > - * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> > - * This software is distributed under GNU GPL v2, 1991 > - * > - * See RFC2474 for a description of the DSCP field within the IP Header. > - * > - * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp > -*/ > #ifndef _XT_DSCP_TARGET_H > #define _XT_DSCP_TARGET_H > -#include <linux/netfilter/xt_dscp.h> > -#include <linux/types.h> > - > -/* target info */ > -struct xt_DSCP_info { > - __u8 dscp; > -}; > > -struct xt_tos_target_info { > - __u8 tos_value; > - __u8 tos_mask; > -}; > +#include <linux/netfilter/xt_dscp.h> > > #endif /* _XT_DSCP_TARGET_H */ > diff --git a/include/uapi/linux/netfilter/xt_dscp.h b/include/uapi/linux/netfilter/xt_dscp.h > index 7594e4df8587..bcfe4afa6351 100644 > --- a/include/uapi/linux/netfilter/xt_dscp.h > +++ b/include/uapi/linux/netfilter/xt_dscp.h > @@ -1,15 +1,17 @@ > /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > -/* x_tables module for matching the IPv4/IPv6 DSCP field > +/* x_tables module for matching/modifying the IPv4/IPv6 DSCP field > * > * (C) 2002 Harald Welte <laforge@gnumonks.org> > + * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> > * This software is distributed under GNU GPL v2, 1991 > * > * See RFC2474 for a description of the DSCP field within the IP Header. > * > + * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp > * xt_dscp.h,v 1.3 2002/08/05 19:00:21 laforge Exp > */ For the sake of history it'd worth to prepend the last two lines with something like: "Original version informations before merging the contents of the files:" > -#ifndef _XT_DSCP_H > -#define _XT_DSCP_H > +#ifndef _UAPI_XT_DSCP_H > +#define _UAPI_XT_DSCP_H In the first four patches you added the _UAPI_ prefix to the header guards while in the next three ones you kept the original ones. Please use one style consistently. > #include <linux/types.h> > > @@ -29,4 +31,14 @@ struct xt_tos_match_info { > __u8 invert; > }; > > -#endif /* _XT_DSCP_H */ > +/* target info */ > +struct xt_DSCP_info { > + __u8 dscp; > +}; > + > +struct xt_tos_target_info { > + __u8 tos_value; > + __u8 tos_mask; > +}; > + > +#endif /* _UAPI_XT_DSCP_H */ > -- > 2.43.5 > > Best regards, Jozsef
2025. 01. 07. 20:23 keltezéssel, Jozsef Kadlecsik írta: > On Tue, 7 Jan 2025, egyszeregy@freemail.hu wrote: > >> From: Benjamin Szőke <egyszeregy@freemail.hu> >> >> Merge xt_DSCP.h to xt_dscp.h header file. > > I think it'd be better worded as "Merge xt_DSCP.h into the xt_dscp.h > header file." (and in the other patches as well). > There will be no any new patchset refactoring anymore just of some cosmetics change. If you like to change it, feel free to modify it in my pacthfiles before the final merging. You can do it as a maintainer. >> Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu> >> --- >> include/uapi/linux/netfilter/xt_DSCP.h | 22 +--------------------- >> include/uapi/linux/netfilter/xt_dscp.h | 20 ++++++++++++++++---- >> 2 files changed, 17 insertions(+), 25 deletions(-) >> >> diff --git a/include/uapi/linux/netfilter/xt_DSCP.h b/include/uapi/linux/netfilter/xt_DSCP.h >> index 223d635e8b6f..fcff72347256 100644 >> --- a/include/uapi/linux/netfilter/xt_DSCP.h >> +++ b/include/uapi/linux/netfilter/xt_DSCP.h >> @@ -1,27 +1,7 @@ >> /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >> -/* x_tables module for setting the IPv4/IPv6 DSCP field >> - * >> - * (C) 2002 Harald Welte <laforge@gnumonks.org> >> - * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> >> - * This software is distributed under GNU GPL v2, 1991 >> - * >> - * See RFC2474 for a description of the DSCP field within the IP Header. >> - * >> - * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp >> -*/ >> #ifndef _XT_DSCP_TARGET_H >> #define _XT_DSCP_TARGET_H >> -#include <linux/netfilter/xt_dscp.h> >> -#include <linux/types.h> >> - >> -/* target info */ >> -struct xt_DSCP_info { >> - __u8 dscp; >> -}; >> >> -struct xt_tos_target_info { >> - __u8 tos_value; >> - __u8 tos_mask; >> -}; >> +#include <linux/netfilter/xt_dscp.h> >> >> #endif /* _XT_DSCP_TARGET_H */ >> diff --git a/include/uapi/linux/netfilter/xt_dscp.h b/include/uapi/linux/netfilter/xt_dscp.h >> index 7594e4df8587..bcfe4afa6351 100644 >> --- a/include/uapi/linux/netfilter/xt_dscp.h >> +++ b/include/uapi/linux/netfilter/xt_dscp.h >> @@ -1,15 +1,17 @@ >> /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >> -/* x_tables module for matching the IPv4/IPv6 DSCP field >> +/* x_tables module for matching/modifying the IPv4/IPv6 DSCP field >> * >> * (C) 2002 Harald Welte <laforge@gnumonks.org> >> + * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> >> * This software is distributed under GNU GPL v2, 1991 >> * >> * See RFC2474 for a description of the DSCP field within the IP Header. >> * >> + * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp >> * xt_dscp.h,v 1.3 2002/08/05 19:00:21 laforge Exp >> */ > > For the sake of history it'd worth to prepend the last two lines with > something like: "Original version informations before merging the contents > of the files:" > This was a question a day ago, what do you like to see in each top of header files's comments. You did not care about it, you was not willing to say something which to be implemented, but now you have new ideas, please it is too late. I will not plan to make any new patchset version just for this new thing which is just a cosmetic change not a critical bugfix. If you like to change it, lets do it, feel free to modify it in my pacthfiles before the final merging or apply your a new patch later. >> -#ifndef _XT_DSCP_H >> -#define _XT_DSCP_H >> +#ifndef _UAPI_XT_DSCP_H >> +#define _UAPI_XT_DSCP_H > > In the first four patches you added the _UAPI_ prefix to the header > guards while in the next three ones you kept the original ones. Please > use one style consistently. > Style consistently is done in the following files: - All of xt_*.h files in uppercase name format (old headers for "target") - All of xt_*.h files in lowercase name format (merged header files) Originally, in these files there was a chaotic state before, it was a painful for my eyes, this is why they got these changes. In ipt_*.h files the original codes got a far enough consistently style before, they was not changed. In my patchsets, It's not my scope/job to make up for the improvements/refactoring of the last 10 years. >> #include <linux/types.h> >> >> @@ -29,4 +31,14 @@ struct xt_tos_match_info { >> __u8 invert; >> }; >> >> -#endif /* _XT_DSCP_H */ >> +/* target info */ >> +struct xt_DSCP_info { >> + __u8 dscp; >> +}; >> + >> +struct xt_tos_target_info { >> + __u8 tos_value; >> + __u8 tos_mask; >> +}; >> + >> +#endif /* _UAPI_XT_DSCP_H */ >> -- >> 2.43.5 >> >> > > Best regards, > Jozsef
On Tue, 7 Jan 2025, Szőke Benjamin wrote: > 2025. 01. 07. 20:23 keltezéssel, Jozsef Kadlecsik írta: > > On Tue, 7 Jan 2025, egyszeregy@freemail.hu wrote: > > > > > From: Benjamin Szőke <egyszeregy@freemail.hu> > > > > > > Merge xt_DSCP.h to xt_dscp.h header file. > > > > I think it'd be better worded as "Merge xt_DSCP.h into the xt_dscp.h > > header file." (and in the other patches as well). > > There will be no any new patchset refactoring anymore just of some > cosmetics change. If you like to change it, feel free to modify it in my > pacthfiles before the final merging. You can do it as a maintainer. We don't modify accepted patches. It rarely happens when time presses and even in that case it is discussed publicly: "sorry, no time to wait for *you* to respin your patch, so I'm going to fix this part, OK?" But there's no time constrain here. So it'd be strange at the minimum if your submitted patches were modified by a maintainer at merging. Believe it or not, I'm just trying to help to get your patches into the best shape. > > > -#ifndef _XT_DSCP_H > > > -#define _XT_DSCP_H > > > +#ifndef _UAPI_XT_DSCP_H > > > +#define _UAPI_XT_DSCP_H > > > > In the first four patches you added the _UAPI_ prefix to the header > > guards while in the next three ones you kept the original ones. Please > > use one style consistently. > > Style consistently is done in the following files: > > - All of xt_*.h files in uppercase name format (old headers for "target") > - All of xt_*.h files in lowercase name format (merged header files) > > Originally, in these files there was a chaotic state before, it was a > painful for my eyes, this is why they got these changes. In ipt_*.h > files the original codes got a far enough consistently style before, > they was not changed. > > In my patchsets, It's not my scope/job to make up for the > improvements/refactoring of the last 10 years. But you are just introducing new inconsistencies: --- a/include/uapi/linux/netfilter/xt_dscp.h +++ b/include/uapi/linux/netfilter/xt_dscp.h ... -#ifndef _XT_DSCP_H -#define _XT_DSCP_H +#ifndef _UAPI_XT_DSCP_H +#define _UAPI_XT_DSCP_H however --- a/include/uapi/linux/netfilter_ipv4/ipt_ecn.h +++ b/include/uapi/linux/netfilter_ipv4/ipt_ecn.h ... #ifndef _IPT_ECN_H #define _IPT_ECN_H Why the "_UAPI_" prefixes are needed in the xt_*.h header files? Best regards, Jozsef
2025. 01. 08. 21:11 keltezéssel, Jozsef Kadlecsik írta: > On Tue, 7 Jan 2025, Szőke Benjamin wrote: > >> 2025. 01. 07. 20:23 keltezéssel, Jozsef Kadlecsik írta: >>> On Tue, 7 Jan 2025, egyszeregy@freemail.hu wrote: >>> >>>> From: Benjamin Szőke <egyszeregy@freemail.hu> >>>> >>>> Merge xt_DSCP.h to xt_dscp.h header file. >>> >>> I think it'd be better worded as "Merge xt_DSCP.h into the xt_dscp.h >>> header file." (and in the other patches as well). >> >> There will be no any new patchset refactoring anymore just of some >> cosmetics change. If you like to change it, feel free to modify it in my >> pacthfiles before the final merging. You can do it as a maintainer. > > We don't modify accepted patches. It rarely happens when time presses and > even in that case it is discussed publicly: "sorry, no time to wait for > *you* to respin your patch, so I'm going to fix this part, OK?" > > But there's no time constrain here. So it'd be strange at the minimum if > your submitted patches were modified by a maintainer at merging. > > Believe it or not, I'm just trying to help to get your patches into the > best shape. > Holyday session is end, i have no time to refactoring and regenerate my patchset in every day, because you have a new idea about cosmetics changes in every next days. (this is why asked you before what you like to get, there was no any answer) If you feel it is need, you can solve it as a maintainer, i know. If you found any critical issue i can fix it later, please start to look for them, but i will not waste my time with this usless commit name and header comment changes, sorry. It is a hobby, i am not a paied Linux developer which is supported by a company for this stuff. As a maintainer you can solve this cosmetics things later in an extra patch or before the merging, lets do it. >>>> -#ifndef _XT_DSCP_H >>>> -#define _XT_DSCP_H >>>> +#ifndef _UAPI_XT_DSCP_H >>>> +#define _UAPI_XT_DSCP_H >>> >>> In the first four patches you added the _UAPI_ prefix to the header >>> guards while in the next three ones you kept the original ones. Please >>> use one style consistently. >> >> Style consistently is done in the following files: >> >> - All of xt_*.h files in uppercase name format (old headers for "target") >> - All of xt_*.h files in lowercase name format (merged header files) >> >> Originally, in these files there was a chaotic state before, it was a >> painful for my eyes, this is why they got these changes. In ipt_*.h >> files the original codes got a far enough consistently style before, >> they was not changed. >> >> In my patchsets, It's not my scope/job to make up for the >> improvements/refactoring of the last 10 years. > > But you are just introducing new inconsistencies: > > --- a/include/uapi/linux/netfilter/xt_dscp.h > +++ b/include/uapi/linux/netfilter/xt_dscp.h > ... > -#ifndef _XT_DSCP_H > -#define _XT_DSCP_H > +#ifndef _UAPI_XT_DSCP_H > +#define _UAPI_XT_DSCP_H > > however > > --- a/include/uapi/linux/netfilter_ipv4/ipt_ecn.h > +++ b/include/uapi/linux/netfilter_ipv4/ipt_ecn.h > ... > #ifndef _IPT_ECN_H > #define _IPT_ECN_H > > Why the "_UAPI_" prefixes are needed in the xt_*.h header files? > Because it is in the UAPI region, don't you hear about the namespace? It is not only relevant for OOP languages. https://www.educative.io/answers/what-is-a-namespace Here is a good any nice example which also got _UAPI prefix in Linux kernel source: https://github.com/torvalds/linux/blob/master/include/uapi/linux/iio/buffer.h By the way, in the API folder, all header should have have had a prefix otherwise it can cause conflict with a same non-uapi header like these: include/net/netfilter /xt_rateest.h -> https://github.com/torvalds/linux/blob/master/include/net/netfilter/xt_rateest.h include/uapi/linux/netfilter /xt_rateest.h -> https://github.com/torvalds/linux/blob/master/include/uapi/linux/netfilter/xt_rateest.h In ipt_*.h, include guards are consistent (where i did any changes) but sure they should have to got that _UAPI prefix also. But this is not the scpoe in my patch, to rafectoring the full netfilter part of the UAPI in Linux, sorry. Please sit down and do it as a maintainer, there were no any relevant refactoring in the past 10 years in this code parts. > Best regards, > Jozsef
diff --git a/include/uapi/linux/netfilter/xt_DSCP.h b/include/uapi/linux/netfilter/xt_DSCP.h index 223d635e8b6f..fcff72347256 100644 --- a/include/uapi/linux/netfilter/xt_DSCP.h +++ b/include/uapi/linux/netfilter/xt_DSCP.h @@ -1,27 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -/* x_tables module for setting the IPv4/IPv6 DSCP field - * - * (C) 2002 Harald Welte <laforge@gnumonks.org> - * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> - * This software is distributed under GNU GPL v2, 1991 - * - * See RFC2474 for a description of the DSCP field within the IP Header. - * - * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp -*/ #ifndef _XT_DSCP_TARGET_H #define _XT_DSCP_TARGET_H -#include <linux/netfilter/xt_dscp.h> -#include <linux/types.h> - -/* target info */ -struct xt_DSCP_info { - __u8 dscp; -}; -struct xt_tos_target_info { - __u8 tos_value; - __u8 tos_mask; -}; +#include <linux/netfilter/xt_dscp.h> #endif /* _XT_DSCP_TARGET_H */ diff --git a/include/uapi/linux/netfilter/xt_dscp.h b/include/uapi/linux/netfilter/xt_dscp.h index 7594e4df8587..bcfe4afa6351 100644 --- a/include/uapi/linux/netfilter/xt_dscp.h +++ b/include/uapi/linux/netfilter/xt_dscp.h @@ -1,15 +1,17 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -/* x_tables module for matching the IPv4/IPv6 DSCP field +/* x_tables module for matching/modifying the IPv4/IPv6 DSCP field * * (C) 2002 Harald Welte <laforge@gnumonks.org> + * based on ipt_FTOS.c (C) 2000 by Matthew G. Marsh <mgm@paktronix.com> * This software is distributed under GNU GPL v2, 1991 * * See RFC2474 for a description of the DSCP field within the IP Header. * + * xt_DSCP.h,v 1.7 2002/03/14 12:03:13 laforge Exp * xt_dscp.h,v 1.3 2002/08/05 19:00:21 laforge Exp */ -#ifndef _XT_DSCP_H -#define _XT_DSCP_H +#ifndef _UAPI_XT_DSCP_H +#define _UAPI_XT_DSCP_H #include <linux/types.h> @@ -29,4 +31,14 @@ struct xt_tos_match_info { __u8 invert; }; -#endif /* _XT_DSCP_H */ +/* target info */ +struct xt_DSCP_info { + __u8 dscp; +}; + +struct xt_tos_target_info { + __u8 tos_value; + __u8 tos_mask; +}; + +#endif /* _UAPI_XT_DSCP_H */