netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
	pabeni@redhat.com, Vladimir Oltean <olteanv@gmail.com>,
	andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
	woojung.huh@microchip.com, o.rempel@pengutronix.de
Subject: Re: [RFC net-next 1/2] net: ethtool: plumb PHY stats to PHY drivers
Date: Wed, 4 Sep 2024 09:20:24 +0200	[thread overview]
Message-ID: <20240904092024.530c54c3@fedora.home> (raw)
In-Reply-To: <20240830113047.10dcee79@kernel.org>

Hi Jakub,

On Fri, 30 Aug 2024 11:30:47 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 30 Aug 2024 10:16:30 +0200 Maxime Chevallier wrote:
> > > +static void
> > > +ethtool_get_phydev_stats(struct net_device *dev,
> > > +			 struct linkstate_reply_data *data)
> > > +{
> > > +	struct phy_device *phydev = dev->phydev;    
> > 
> > This would be a very nice spot to use the new
> > ethnl_req_get_phydev(), if there are multiple PHYs on that device.
> > Being able to access the stats individually can help
> > troubleshoot HW issues.
> >   
> > > +static void
> > > +ethtool_get_phydev_stats(struct net_device *dev, struct stats_reply_data *data)
> > > +{
> > > +	struct phy_device *phydev = dev->phydev;    
> > 
> > Here as well, but that's trickier, as the MAC can override the PHY
> > stats, but it doesn't know which PHY were getting the stats from.
> > 
> > Maybe we could make so that when we pass a phy_index in the netlink
> > command, we don't allow the mac to override the phy stats ? Or better,
> > don't allow the mac to override these stats and report the MAC-reported
> > PHY stats alongside the PHY-reported stats ?  
> 
> Maybe we can flip the order of querying regardless of the PHY that's
> targeted? Always query the MAC first and then the PHY, so that the
> PHY can override. Presumably the PHY can always provide more detailed
> stats than the MAC (IOW if it does provide stats they will be more
> accurate).

I think that could work indeed, good point.

[...]

> > I'm all in for getting the PHY stats from netlink though :)  
> 
> Great! FWIW I'm not sure what the status of these patches is.
> I don't know much about PHYs.
> I wrote them to help Oleksij out with the "netlink parts".
> I'm not sure how much I convinced Andrew about the applicability.
> And I don't know if this is enough for Oleksij to take it forward.
> So in the unlikely even that you have spare cycles and a PHY you can
> test with, do not hesitate to take these, rework, reset the author 
> and repost... :)

I do have some hardware I can test this on, and I'm starting to get
familiar with netlink :) I can give it a try, however I can't guarantee
as of right now that I'll be able to send anything before net-next
closes. I'll ping here if I start moving forward with this :)

Thanks,

Maxime

  reply	other threads:[~2024-09-04  7:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-29 17:43 [RFC net-next 0/2] net: ethtool: add phy(dev) specific stats over netlink Jakub Kicinski
2024-08-29 17:43 ` [RFC net-next 1/2] net: ethtool: plumb PHY stats to PHY drivers Jakub Kicinski
2024-08-29 18:10   ` Oleksij Rempel
2024-08-29 19:13     ` Jakub Kicinski
2024-08-30  8:16   ` Maxime Chevallier
2024-08-30 18:30     ` Jakub Kicinski
2024-09-04  7:20       ` Maxime Chevallier [this message]
2024-09-04  8:05         ` Oleksij Rempel
2024-09-04  8:09           ` Maxime Chevallier
2024-09-02 15:08   ` Vladimir Oltean
2024-08-29 17:43 ` [RFC net-next 2/2] net: ethtool: add phy(dev) specific stats over netlink Jakub Kicinski
2024-08-29 18:47   ` Andrew Lunn
2024-08-29 19:23     ` Jakub Kicinski

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=20240904092024.530c54c3@fedora.home \
    --to=maxime.chevallier@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=woojung.huh@microchip.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).