[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240104151242.52fa8cb4@kernel.org>
Date: Thu, 4 Jan 2024 15:12:42 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com, Andrew Lunn
<andrew@...n.ch>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
<pabeni@...hat.com>, Russell King <linux@...linux.org.uk>,
linux-arm-kernel@...ts.infradead.org, Christophe Leroy
<christophe.leroy@...roup.eu>, Herve Codina <herve.codina@...tlin.com>,
Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit
<hkallweit1@...il.com>, Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>, Jesse Brandeburg
<jesse.brandeburg@...el.com>, Jonathan Corbet <corbet@....net>, Marek
Behún <kabel@...nel.org>, Piergiorgio Beruto
<piergiorgio.beruto@...il.com>, Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>, Simon Horman
<horms@...nel.org>
Subject: Re: [PATCH net-next v5 01/13] net: phy: Introduce ethernet link
topology representation
On Thu, 21 Dec 2023 19:00:34 +0100 Maxime Chevallier wrote:
> @@ -2441,6 +2442,7 @@ struct net_device {
> #if IS_ENABLED(CONFIG_CGROUP_NET_PRIO)
> struct netprio_map __rcu *priomap;
> #endif
> + struct phy_link_topology link_topo;
Perhaps others would disagree but can we make this a pointer instead?
Only allocate it on demand, when first PHY gets attached?
Both saves space and netdevice.h will no longer need to know the
definition of the struct.
Complete noob question but I thought PHYs get attached at ndo_open
time for drivers, don't they? We shouldn't want to re-ID in that case.
> struct phy_device *phydev;
> struct sfp_bus *sfp_bus;
> struct lock_class_key *qdisc_tx_busylock;
> @@ -10872,6 +10873,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> #ifdef CONFIG_NET_SCHED
> hash_init(dev->qdisc_hash);
> #endif
> + phy_link_topo_init(&dev->link_topo);
> +
> dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
> setup(dev);
>
I think you're missing a call to xa_destroy() somewhere, no?
Powered by blists - more mailing lists