netdev - Re: [PATCH net-next v2] net: mvpp2: add ethtool GOP statistics

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-next>] [day] [month] [year] [list]

Message-ID: <20171106234523.7e170a71@xps13>
Date:   Mon, 6 Nov 2017 23:45:23 +0100
From:   Miquel RAYNAL <miquel.raynal@...e-electrons.com>
To:     Stefan Chulski <stefanc@...vell.com>
Cc:     Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        Antoine Tenart <antoine.tenart@...e-electrons.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2] net: mvpp2: add ethtool GOP statistics

Hi Stefan,

+David Miller/Net ML

> > @@ -6844,6 +7023,10 @@ static int mvpp2_open(struct net_device
> > *dev)
> > 
> >  	mvpp2_start_dev(port);
> > 
> > +	/* Start hardware statistics gathering */
> > +	queue_delayed_work(priv->stats_queue, &priv->stats_work,
> > +			   MVPP2_MIB_COUNTERS_STATS_DELAY);
> > +
> >  	return 0;
> > 
> >  err_free_link_irq:
> > @@ -6888,6 +7071,9 @@ static int mvpp2_stop(struct net_device *dev)
> >  	mvpp2_cleanup_rxqs(port);
> >  	mvpp2_cleanup_txqs(port);
> > 
> > +	cancel_delayed_work_sync(&priv->stats_work);
> > +	flush_workqueue(priv->stats_queue);
> > +  
> 
> Hi Miquel,
> 
> I think there are bug here.
> priv is common for all ports on same CPN and they have same
> priv->stats_work.
> 
> For example on A7K board with 3 Ports. queue_delayed_work and
> cancel_delayed_work_sync called for each port stop and start
> procedure. For example:
> If Port0 and Port1 were started, then if only Port0 stopped, delayed
> work would be canceled for both ports.


Thanks for spotting it, you are right this is a bug since I moved
starting/stopping the queues in the opening and close procedure of the
ports (to avoid using CPU time while no interface is actually up).

Maybe I should have a work per port, it would be easier to
handle.

Thank you,
Miquèl