All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: "Szőke Benjamin" <egyszeregy@freemail.hu>
Cc: Florian Westphal <fw@strlen.de>,
	pablo@netfilter.org, lorenzo@kernel.org, daniel@iogearbox.net,
	leitao@debian.org, amiculas@cisco.com, kadlec@netfilter.org,
	davem@davemloft.net, dsahern@kernel.org, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] netfilter: uapi: Merge xt_*.h/c and ipt_*.h which has same name.
Date: Thu, 2 Jan 2025 13:35:56 +0100	[thread overview]
Message-ID: <20250102123556.GC3344@breakpoint.cc> (raw)
In-Reply-To: <4ad8fb04-b2d6-493b-978c-7dea46fdc623@freemail.hu>

Szőke Benjamin <egyszeregy@freemail.hu> wrote:
> 2025. 01. 01. 23:46 keltezéssel, Florian Westphal írta:
> > egyszeregy@freemail.hu <egyszeregy@freemail.hu> wrote:
> > >   /* match info */
> > > -struct xt_dscp_info {
> > > +struct xt_dscp_match_info {
> > 
> > To add to what Jan already pointed out, such renames
> > break UAPI, please don't do this.
> > 
> > It could be done with compat ifdef'ry but I think its rather ugly,
> > better to keep all uapi structure names as-is.
> 
> If i keep the original, maybe one of them will be in conflict between
> "match" and "target" structs name if i remember well (they go the same
> text).

Did not find an example.  Can you please point me to one?

> By the way original structs name are absolutely not following any
> good clean coding, they will be still ugly and they are hard to understand
> quickly in the code, what goes for "target" and what goes fot "match" codes.
> Why is it bad to step forward and accept a breaking change to gets a better
> clean code?

Breaking changes are not acceptable.

> > >   MODULE_LICENSE("GPL");
> > >   MODULE_AUTHOR("Marc Boucher <marc@mbsi.ca>");
> > > -MODULE_DESCRIPTION("Xtables: TCP MSS match");
> > > +MODULE_DESCRIPTION("Xtables: TCP Maximum Segment Size (MSS) adjustment/match");
> > >   MODULE_ALIAS("ipt_tcpmss");
> > >   MODULE_ALIAS("ip6t_tcpmss");
> > > +MODULE_ALIAS("ipt_TCPMSS");
> > > +MODULE_ALIAS("ip6t_TCPMSS");
> > 
> > I think you should add MODULE_ALIAS("xt_TCPMSS") just in case, same
> > for all other merged (== 'removed') module names, to the respective
> > match (preserved) modules.
> 
> Do you mean in all of xt_*.c source, it can be appended by its own
> MODULE_ALIAS("xt_TCPMSS"), MODULE_ALIAS("xt_RATEEST") ... and so on? Can be
> kept old MODULE_ALIAS() names  or they can be removed?

'modprobe xt_FOO' should continue to work, so if xt_FOO was merged into
xt_foo, then 'modprobe xt_FOO' should load xt_foo.

Makes sense?

Same reason as why we have the ipt_tcpmss etc. aliases, it should load
the xt_ module which does provide the relevant functionality.

So yes, please keep all existing aliases.

  reply	other threads:[~2025-01-02 12:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-01 19:20 [PATCH] netfilter: uapi: Merge xt_*.h/c and ipt_*.h which has same name egyszeregy
2025-01-01 22:30 ` kernel test robot
2025-01-01 22:37 ` Jan Engelhardt
2025-01-01 22:46 ` Florian Westphal
2025-01-01 23:47   ` Szőke Benjamin
2025-01-02 12:35     ` Florian Westphal [this message]
2025-01-02  7:55   ` David Laight
2025-01-02 12:23     ` Florian Westphal
2025-01-02 13:34 ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250102123556.GC3344@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=amiculas@cisco.com \
    --cc=coreteam@netfilter.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=egyszeregy@freemail.hu \
    --cc=horms@kernel.org \
    --cc=kadlec@netfilter.org \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.