netdev - Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

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: <1375902775.2644.98.camel@deadeye.wl.decadent.org.uk>
Date:	Wed, 7 Aug 2013 21:12:55 +0200
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Claudiu Manoil <claudiu.manoil@...escale.com>
CC:	<netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause
 frame

On Wed, 2013-08-07 at 13:24 +0300, Claudiu Manoil wrote:
> Allow Rx/Tx pause frame configuration via ethtool.
> The gfar devices feature link autonegotioation by default.

So the MAC configuration bits are actually copied to the PHY autoneg
basic page, and then the PHY autoneg result is automatically used by the
MAC?

This is of course possible to do in hardware, but... since this MAC is
not smart enough to ignore pause settings when running in half-duplex
mode, I seriously doubt it is doing all this by itself.

> The device is being configured with the new pause frame
> parameters if the link is up, depending on link duplex (no
> pause frames for half-duplex links), or during link autoneg
> (see adjust_link()).
[...]
> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> @@ -535,6 +535,34 @@ static int gfar_sringparam(struct net_device *dev,
>  	return err;
>  }
>  
> +static void gfar_gpauseparam(struct net_device *dev,
> +			     struct ethtool_pauseparam *pause)
> +{
> +	struct gfar_private *priv = netdev_priv(dev);
> +
> +	pause->autoneg = AUTONEG_ENABLE;
> +	if (priv->rx_pause)
> +		pause->rx_pause = 1;
> +	if (priv->tx_pause)
> +		pause->tx_pause = 1;
> +}
> +
> +static int gfar_spauseparam(struct net_device *dev,
> +			    struct ethtool_pauseparam *pause)
> +{
> +	struct gfar_private *priv = netdev_priv(dev);
> +	struct phy_device *phydev = priv->phydev;

You need to reject an unsupported setting of pause->autoneg here.

Ben.

> +	priv->rx_pause = !!pause->rx_pause;
> +	priv->tx_pause = !!pause->tx_pause;
> +
> +	/* update h/w settings, if link is up */
> +	if (phydev && phydev->link)
> +		gfar_configure_pause(priv, !!phydev->duplex);
> +
> +	return 0;
> +}
> +
>  int gfar_set_features(struct net_device *dev, netdev_features_t features)
>  {
>  	struct gfar_private *priv = netdev_priv(dev);
[...]

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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