netdev - Re: [PATCH] r8169: Don't update statistics counters when interface is down

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:	Tue, 3 Feb 2009 22:46:41 +0100
From:	Francois Romieu <romieu@...zoreil.com>
To:	Ivan Vecera <ivecera@...hat.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] r8169: Don't update statistics counters when interface
	is down

Ivan Vecera <ivecera@...hat.com> :
> Some Realtek chips (RTL8169sb/8110sb in my case) are unable to retrieve
> ethtool statistics when the interface is down. The process stays in
> endless loop in rtl8169_get_ethtool_stats. This is because these chips
> need to have receiver enabled (CmdRxEnb bit in ChipCmd register) that is
> cleared when the interface is going down. It's better to update statistics
> only when the interface is up and otherwise return copy of statistics
> grabbed when the interface was up (in rtl8169_close).

It makes sense.

[...]
> +static void rtl8169_update_counters(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
>  	void __iomem *ioaddr = tp->mmio_addr;
>  	struct rtl8169_counters *counters;
>  	dma_addr_t paddr;
>  	u32 cmd;
> +	int wait = 1000;
>  
> -	ASSERT_RTNL();
> +	if (!netif_running(dev))
> +		return;


int dev_close(struct net_device *dev)
{
[...]
	clear_bit(__LINK_STATE_START, &dev->state);
        -> netif_running is not true anymore
[...]
	if (ops->ndo_stop)
		ops->ndo_stop(dev);

-> Calling rtl8169_update_counters in rtl8169_close looks like a no-op.

Aside from that, I do not understand from the description the need for
the "while (wait--)" loop.

Aside from the aside, the diff can probably be shorter if a local "counters"
variable is kept in the ethtool function (nit).

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html