[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231219085557.GB811967@kernel.org>
Date: Tue, 19 Dec 2023 08:55:57 +0000
From: Simon Horman <horms@...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>, Jakub Kicinski <kuba@...nel.org>,
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>
Subject: Re: [PATCH net-next v4 07/13] net: ethtool: Introduce a command to
list PHYs on an interface
On Fri, Dec 15, 2023 at 06:12:29PM +0100, Maxime Chevallier wrote:
> As we have the ability to track the PHYs connected to a net_device
> through the link_topology, we can expose this list to userspace. This
> allows userspace to use these identifiers for phy-specific commands and
> take the decision of which PHY to target by knowing the link topology.
>
> Add PHY_GET and PHY_DUMP, which can be a filtered DUMP operation to list
> devices on only one interface.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
Hi Maxime,
some minor feedback from my side.
> +static int ethnl_phy_parse_request(struct ethnl_req_info *req_base,
> + struct nlattr **tb)
> +{
> + struct phy_req_info *req_info = PHY_REQINFO(req_base);
> + struct phy_link_topology *topo = &req_base->dev->link_topo;
> + struct phy_device_node *pdn;
nit: Please consider arranging local variables in reverse xmas tree order -
longest line to shortest.
> +
> + if (!req_base->phydev)
> + return 0;
> +
> + pdn = xa_load(&topo->phys, req_base->phydev->phyindex);
> + memcpy(&req_info->pdn, pdn, sizeof(*pdn));
> +
> + return 0;
> +}
...
> +static int ethnl_phy_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
> + struct netlink_callback *cb)
> +{
> + struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
> + struct phy_req_info *pri = ctx->phy_req_info;
> + struct phy_device_node *pdn;
> + unsigned long index = 1;
> + void *ehdr;
> + int ret;
> +
> + pri->base.dev = dev;
> +
> + xa_for_each(&dev->link_topo.phys, index, pdn) {
> + ehdr = ethnl_dump_put(skb, cb,
> + ETHTOOL_MSG_PHY_GET_REPLY);
> + if (!ehdr) {
> + ret = -EMSGSIZE;
> + break;
> + }
> +
> + ret = ethnl_fill_reply_header(skb, dev,
> + ETHTOOL_A_PHY_HEADER);
> + if (ret < 0) {
> + genlmsg_cancel(skb, ehdr);
> + break;
> + }
> +
> + memcpy(&pri->pdn, pdn, sizeof(*pdn));
> + ret = ethnl_phy_fill_reply(&pri->base, skb);
> +
> + genlmsg_end(skb, ehdr);
> + }
> +
> + return ret;
I'm unsure if it can happen, but if the loop runs zero times then
ret will be used uninitialised here.
Flagged by Smatch.
> +}
...
Powered by blists - more mailing lists