[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8120c842-2205-415e-ab78-24200efe72a3@lunn.ch>
Date: Mon, 18 Nov 2024 16:05:53 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Josua Mayer <josua@...id-run.com>
Cc: Jakub Kicinski <kuba@...nel.org>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Mor Nagli <mor.nagli@...id-run.com>
Subject: Re: [PATCH net-next v3] net: dsa: mv88e6xxx: control mdio bus-id
truncation for long paths
On Mon, Nov 18, 2024 at 02:57:45PM +0000, Josua Mayer wrote:
> Am 13.05.24 um 17:32 schrieb Jakub Kicinski:
> > On Tue, 7 May 2024 12:03:31 +0000 Josua Mayer wrote:
> >>> The idea and implementation is reasonable but this could affect other drivers than mv88e6xxx, why not move that logic to mdiobus_register() and tracking the truncation index globally within the MDIO bus layer?
> >> Conceptually I agree, it would be nice to have a centralized
> >> solution to this problem, it probably can occur in multiple places.
> >>
> >> My reasoning is that solving the problem within a single driver
> >> is a much smaller task, especially for sporadic contributors
> >> who lack a deep understanding for how all layers interact.
> >>
> >> Perhaps agreeing on a good solution within this driver
> >> can inform a more general solution to be added later.
> > I agree with Florian, FWIW. The choice of how to truncate is a bit
> > arbitrary, if core does it at least it will be consistent.
>
> Very true.
>
> However the names themselves are so far generated by each driver,
> if mdiobus_register should define truncation behaviour,
> then the name format must be passed as an argument, too.
>
> How about adding new properties to mii_bus?
> But then how to pass the variable arguments ....
>
> Is it acceptable to have variadic function?:
>
> int __mdiobus_register(struct mii_bus *bus, struct module *owner, const char *name_format, ...);
Probably not. The format string needs to be fully under control and
constant because otherwise it can be abused. If you look around, you
will see patches converting invocations of variadic functions from
(foo), to ("%s", foo), because if foo can be under user control, it
might contain "%s%s%d%x%lld" causing it to access stuff never passed
to it.
Andrew
Powered by blists - more mailing lists