netdev - Re: [PATCH v2 net-next] r8169: Add values missing in @get

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: <20150821220756.GI19683@calimero.vinschen.de>
Date:	Sat, 22 Aug 2015 00:07:56 +0200
From:	Corinna Vinschen <vinschen@...hat.com>
To:	Francois Romieu <romieu@...zoreil.com>
Cc:	netdev@...r.kernel.org, Hayes Wang <hayeswang@...ltek.com>,
	Ivan Vecera <ivecera@...hat.com>,
	David Miller <davem@...emloft.net>,
	nic_swsd <nic_swsd@...ltek.com>
Subject: Re: [PATCH v2 net-next] r8169: Add values missing in @get_stats64
 from HW counters

On Aug 21 21:39, Francois Romieu wrote:
> Corinna Vinschen <vinschen@...hat.com> :
> [...]
> > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> > index f790f61..f26a48d 100644
> > --- a/drivers/net/ethernet/realtek/r8169.c
> > +++ b/drivers/net/ethernet/realtek/r8169.c
> [...]
> > @@ -2179,6 +2191,47 @@ static int rtl8169_get_sset_count(struct net_device *dev, int sset)
> >  	}
> >  }
> >  
> > +DECLARE_RTL_COND(rtl_reset_counters_cond)
> > +{
> > +	void __iomem *ioaddr = tp->mmio_addr;
> > +
> > +	return RTL_R32(CounterAddrLow) & CounterReset;
> > +}
> > +
> > +static void rtl8169_reset_counters(struct net_device *dev)
> > +{
> 
> rtl8169_reset_counters duplicates most of rtl8169_update_counters. Please
> factor out the dma_alloc + parametrized CounterAddrLow write + cleanup.

Ok, will do.

> > +	rtl8169_reset_counters(dev);
> > +
> > +	rtl8169_update_counters(dev);
> 
> 
> The code should propagate failure when both rtl8169_reset_counters and
> rtl8169_update_counters fail.

This one I don't understand.  Neither failing to reset the counters nor
failing to update the counters is fatal for the driver.  So far the
(unchanged) rtl8169_update_counters doesn't even print a log message,
while a failing reset in rtl8169_reset_counters now does.

Why is that not sufficent?


Thanks,
Corinna

Content of type "application/pgp-signature" skipped