netdev - RE: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support

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]

Message-ID: <SN1PR11MB04465D50382C3F7438DA0FADEC660@SN1PR11MB0446.namprd11.prod.outlook.com>
Date:   Wed, 13 Feb 2019 02:39:49 +0000
From:   <Tristram.Ha@...rochip.com>
To:     <andrew@...n.ch>
CC:     <sergio.paracuellos@...il.com>, <f.fainelli@...il.com>,
        <pavel@....cz>, <UNGLinuxDriver@...rochip.com>,
        <netdev@...r.kernel.org>
Subject: RE: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter
 reading support

> > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool
> freeze)
> > +{
> > +	struct ksz_port *p = &dev->ports[port];
> > +	u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
> 
> Reverse Christmas tree.

There was a checkpatch.pl patch in 2016 that tried to check this, but it was never accepted?

> > +		/* read only dropped counters when link is not up */
> > +		if (p->link_just_down)
> > +			p->link_just_down = 0;
> > +		else if (!p->phydev.link)
> > +			mib->cnt_ptr = dev->reg_mib_cnt;
> 
> This link_just_down stuff is not clear at all. Why can the drop
> counters not be read when the link is up?

All of the MIB counters, except some that may be marked by driver, do not get updated when the link is down, so it is a waste of time to read them.

My intention is the driver eventually reads the MIB counters at least every second or faster so that the ethtool API called to show MIB counters gets them from memory rather than starting a read operation.  For now that API is called from user space with the ethtool utility, so it may not be called too often and too fast.  But theoretically that API can be called from a program continually.

For simple switches that do not need to do anything special the MIB read operation does not cause any issue except CPU load, for more complicate switches that need to do some background operations too many read operation can affect some critical functions.