Re: [PATCH net 3/3] r8169: increase the lifespan of the hardware counters dump area.

[Corinna Vinschen <vinschen@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]

On Sep  5 14:18, romieu@fr.zoreil.com wrote:
> From: Francois Romieu <romieu@fr.zoreil.com>
> 
> net/core/net-sysfs.c::netstat_show retrieves stats with spinlock held.
> 
> This change avoids sleepable allocation and performs some housekeeping:
> - receive ring, transmit ring and counters dump area allocation failures
>   are all considered fatal during open()
> - netif_warn is now redundant with rtl_reset_counters_cond built-in
>   failure message.
> - rtl_reset_counters_cond induced failures in open() are also considered
>   fatal: it takes acceptable work to unwind comfortably.

Why?  The counter reset is not a fatal condition for the operation of
the NIC.  Even if the counters show incorrect values, the normal
operation of the NIC is not affected.  And to top that off, even if
resetting the counters actually doesn't work, the driver keeps the
offset values, so a failed reset won't even harm the statistics.  It
just doesn't make sense to break the NIC operation for such a minor
problem.  And worse:

> -static bool rtl8169_reset_counters(struct net_device *dev)
> +static int rtl8169_reset_counters(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
> -	struct rtl8169_counters *counters;
> -	dma_addr_t paddr;
> -	bool ret = true;
>  
>  	/*
>  	 * Versions prior to RTL_GIGA_MAC_VER_19 don't support resetting the
>  	 * tally counters.
>  	 */
>  	if (tp->mac_version < RTL_GIGA_MAC_VER_19)
> -		return true;
> -
> -	counters = rtl8169_map_counters(dev, &paddr, CounterReset);
> -	if (!counters)
> -		return false;
> -
> -	if (!rtl_udelay_loop_wait_low(tp, &rtl_reset_counters_cond, 10, 1000))
> -		ret = false;
> -
> -	rtl8169_unmap_counters(dev, paddr, counters);
> +		return -EINVAL;

This returns -EINVAL even for older chip versions which are not capable
of resetting the counters.  The result is, this driver will not work at
all on chip versions prior to RTL_GIGA_MAC_VER_19 anymore, because
rtl_open will always fail.

Other then that, I can test the patch probably tomorrow.


Corinna

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]