netdev - Re: [PATCH V2] Add ethtool to mii advertisment conversion helpers

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

Message-Id: <20111120.162617.1715993269904044868.davem@davemloft.net>
Date:	Sun, 20 Nov 2011 16:26:17 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	mcarlson@...adcom.com, netdev@...r.kernel.org, mchan@...adcom.com
Subject: Re: [PATCH V2] Add ethtool to mii advertisment conversion helpers

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Sun, 20 Nov 2011 22:11:37 +0100

> For reference, the revert on tg3 :
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 024ca1d..365cd47 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -3594,7 +3594,15 @@ static int tg3_phy_autoneg_cfg(struct tg3 *tp, u32 advertise, u32 flowctrl)
>  	u32 val, new_adv;
>  
>  	new_adv = ADVERTISE_CSMA;
> -	new_adv |= ethtool_adv_to_mii_100bt(advertise);
> +	if (advertise & ADVERTISED_10baseT_Half)
> +		new_adv |= ADVERTISE_10HALF;
> +	if (advertise & ADVERTISED_10baseT_Full)
> +		new_adv |= ADVERTISE_10FULL;
> +	if (advertise & ADVERTISED_100baseT_Half)
> +		new_adv |= ADVERTISE_100HALF;
> +	if (advertise & ADVERTISED_100baseT_Full)
> +		new_adv |= ADVERTISE_100FULL;
> +
>  	new_adv |= tg3_advert_flowctrl_1000T(flowctrl);
>  

Well, for one thing the flow control handling is subtly changed here.

ethtool_adv_to_mii_100bt() does it's own setting of the flow control
advertisement bits.  It sets ADVERTISE_PAUSE_CAP and
ADVERTISE_PAUSE_ASYM.

But here in tg3, tg3_advert_flowctrl_1000T() has it's own seperate
logic for setting those bits.

I cannot say for certain that this is what it causing your problems,
but it is clear now that these transformations were not done carefully
enough.

--
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