netdev - Re: [PATCH 1/1] ethtool : added get_phy_stats,get_strings,get_sset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Mar 2017 19:26:57 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Thomas Scariah <thomasscariah@...il.com>, nsekhar@...com,
        grygorii.strashko@...com, davem@...emloft.net,
        drivshin@...worx.com, mugunthanvnm@...com,
        ivan.khoronzhuk@...aro.org, thomas.scariah@...man.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] ethtool : added
 get_phy_stats,get_strings,get_sset_count

> >  static int cpsw_get_sset_count(struct net_device *ndev, int sset)
> >  {
> > +	struct cpsw_priv *priv = netdev_priv(ndev);
> > +	int slave_no = cpsw_slave_index(priv);
> > +	int count;
> > +
> >  	switch (sset) {
> >  	case ETH_SS_STATS:
> > -		return CPSW_STATS_LEN;
> > +		count = CPSW_STATS_LEN;
> > +		count += phy_ethtool_get_sset_count(priv->slaves[slave_no].phy,
> > +						    sset);
> > +		return count;
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> 
> We already have a way to obtain PHY specific statistics through the
> ETH_SS_PHY_STATS string set, cannot we use that here too? It certainly
> makes it easier to overlay cpsw statistics with PHY device statistics,
> but when Andrew added support for that, AFAIR this was made
> intentionally separate.

I don't particularly like this. It makes the cpsw driver different to
all other drivers, in that it returns both MAC and PHY statistics.

If you are going to do this, please do it a higher level so that it
applies to all Ethernet drivers, not just cpsw.

But personally, i don't see the point of this change. What is wrong
with ethtool --phy-statistics?

     Andrew