diff mbox

[net-next-2.6] sfc: Implement NETIF_F_LOOPBACK

Message ID 1304640751.2857.34.camel@bwh-desktop
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Ben Hutchings May 6, 2011, 12:12 a.m. UTC 
We already have comprehensive support for loopback testing, so pick
the nearest mode and run with it.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
David, this depends on Mahesh's patch to define NETIF_F_LOOPBACK
<http://patchwork.ozlabs.org/patch/94190/>.  Although you've marked that
as RFC, I think it is ready to apply.

Ben.

 drivers/net/sfc/efx.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

Comments

Michał Mirosław May 9, 2011, 7:36 a.m. UTC | #1 
On Fri, May 06, 2011 at 01:12:31AM +0100, Ben Hutchings wrote:
> We already have comprehensive support for loopback testing, so pick
> the nearest mode and run with it.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> David, this depends on Mahesh's patch to define NETIF_F_LOOPBACK
> <http://patchwork.ozlabs.org/patch/94190/>.  Although you've marked that
> as RFC, I think it is ready to apply.
> 
> Ben.
> 
>  drivers/net/sfc/efx.c |   24 ++++++++++++++++++++++--
>  1 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
> index 38a55e9..b7bfb49 100644
> --- a/drivers/net/sfc/efx.c
> +++ b/drivers/net/sfc/efx.c
> @@ -1884,6 +1884,25 @@ static int efx_set_features(struct net_device *net_dev, u32 data)
>  	if (net_dev->features & ~data & NETIF_F_NTUPLE)
>  		efx_filter_clear_rx(efx, EFX_FILTER_PRI_MANUAL);
>  
> +	/* Toggle loopback if required */
> +	if ((net_dev->features ^ data) & NETIF_F_LOOPBACK) {
> +		enum efx_loopback_mode old_mode;
> +		int rc;
> +
> +		mutex_lock(&efx->mac_lock);
> +		old_mode = efx->loopback_mode;
> +		if (data & NETIF_F_LOOPBACK)
> +			efx->loopback_mode = __ffs(efx->loopback_modes);
> +		else
> +			efx->loopback_mode = LOOPBACK_NONE;
> +		rc = __efx_reconfigure_port(efx);
> +		if (rc)
> +			efx->loopback_mode = old_mode;
> +		mutex_unlock(&efx->mac_lock);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	return 0;
>  }

If other features than NETIF_F_LOOPBACK were changed correctly, this should
update dev->features of what changed before error return. If not, device's
state will diverge to what is in dev->features.

> @@ -2472,8 +2491,9 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
>  	net_dev->vlan_features |= (NETIF_F_ALL_CSUM | NETIF_F_SG |
>  				   NETIF_F_HIGHDMA | NETIF_F_ALL_TSO |
>  				   NETIF_F_RXCSUM);
> -	/* All offloads can be toggled */
> -	net_dev->hw_features = net_dev->features & ~NETIF_F_HIGHDMA;
> +	/* All offloads can be toggled, and so can loopback */
> +	net_dev->hw_features = ((net_dev->features & ~NETIF_F_HIGHDMA) |
> +				NETIF_F_LOOPBACK);
>  	efx = netdev_priv(net_dev);
>  	pci_set_drvdata(pci_dev, efx);
>  	SET_NETDEV_DEV(net_dev, &pci_dev->dev);

You can remove the '& ~NETIF_F_HIGHDMA' part, as it's now allowed to be
changed (commit fa2bd7ff9247f4218dfc907db14d000cd7edd862).

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings May 9, 2011, 1:47 p.m. UTC | #2 
On Mon, 2011-05-09 at 09:36 +0200, Michał Mirosław wrote:
> On Fri, May 06, 2011 at 01:12:31AM +0100, Ben Hutchings wrote:
> > We already have comprehensive support for loopback testing, so pick
> > the nearest mode and run with it.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > David, this depends on Mahesh's patch to define NETIF_F_LOOPBACK
> > <http://patchwork.ozlabs.org/patch/94190/>.  Although you've marked that
> > as RFC, I think it is ready to apply.
> > 
> > Ben.
> > 
> >  drivers/net/sfc/efx.c |   24 ++++++++++++++++++++++--
> >  1 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
> > index 38a55e9..b7bfb49 100644
> > --- a/drivers/net/sfc/efx.c
> > +++ b/drivers/net/sfc/efx.c
> > @@ -1884,6 +1884,25 @@ static int efx_set_features(struct net_device *net_dev, u32 data)
> >  	if (net_dev->features & ~data & NETIF_F_NTUPLE)
> >  		efx_filter_clear_rx(efx, EFX_FILTER_PRI_MANUAL);
> >  
> > +	/* Toggle loopback if required */
> > +	if ((net_dev->features ^ data) & NETIF_F_LOOPBACK) {
> > +		enum efx_loopback_mode old_mode;
> > +		int rc;
> > +
> > +		mutex_lock(&efx->mac_lock);
> > +		old_mode = efx->loopback_mode;
> > +		if (data & NETIF_F_LOOPBACK)
> > +			efx->loopback_mode = __ffs(efx->loopback_modes);
> > +		else
> > +			efx->loopback_mode = LOOPBACK_NONE;
> > +		rc = __efx_reconfigure_port(efx);
> > +		if (rc)
> > +			efx->loopback_mode = old_mode;
> > +		mutex_unlock(&efx->mac_lock);
> > +		if (rc)
> > +			return rc;
> > +	}
> > +
> >  	return 0;
> >  }
> 
> If other features than NETIF_F_LOOPBACK were changed correctly, this should
> update dev->features of what changed before error return. If not, device's
> state will diverge to what is in dev->features.

In general __efx_reconfigure_port() might fail because PHYs are tricky
beasts, but for internal loopback that isn't an issue.  So this is very
unlikely to fail.  But I suppose I should move it to the top of the
function.

> > @@ -2472,8 +2491,9 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
> >  	net_dev->vlan_features |= (NETIF_F_ALL_CSUM | NETIF_F_SG |
> >  				   NETIF_F_HIGHDMA | NETIF_F_ALL_TSO |
> >  				   NETIF_F_RXCSUM);
> > -	/* All offloads can be toggled */
> > -	net_dev->hw_features = net_dev->features & ~NETIF_F_HIGHDMA;
> > +	/* All offloads can be toggled, and so can loopback */
> > +	net_dev->hw_features = ((net_dev->features & ~NETIF_F_HIGHDMA) |
> > +				NETIF_F_LOOPBACK);
> >  	efx = netdev_priv(net_dev);
> >  	pci_set_drvdata(pci_dev, efx);
> >  	SET_NETDEV_DEV(net_dev, &pci_dev->dev);
> 
> You can remove the '& ~NETIF_F_HIGHDMA' part, as it's now allowed to be
> changed (commit fa2bd7ff9247f4218dfc907db14d000cd7edd862).

OK.

Ben.
diff mbox

Patch

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 38a55e9..b7bfb49 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1884,6 +1884,25 @@  static int efx_set_features(struct net_device *net_dev, u32 data)
 	if (net_dev->features & ~data & NETIF_F_NTUPLE)
 		efx_filter_clear_rx(efx, EFX_FILTER_PRI_MANUAL);
 
+	/* Toggle loopback if required */
+	if ((net_dev->features ^ data) & NETIF_F_LOOPBACK) {
+		enum efx_loopback_mode old_mode;
+		int rc;
+
+		mutex_lock(&efx->mac_lock);
+		old_mode = efx->loopback_mode;
+		if (data & NETIF_F_LOOPBACK)
+			efx->loopback_mode = __ffs(efx->loopback_modes);
+		else
+			efx->loopback_mode = LOOPBACK_NONE;
+		rc = __efx_reconfigure_port(efx);
+		if (rc)
+			efx->loopback_mode = old_mode;
+		mutex_unlock(&efx->mac_lock);
+		if (rc)
+			return rc;
+	}
+
 	return 0;
 }
 
@@ -2472,8 +2491,9 @@  static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
 	net_dev->vlan_features |= (NETIF_F_ALL_CSUM | NETIF_F_SG |
 				   NETIF_F_HIGHDMA | NETIF_F_ALL_TSO |
 				   NETIF_F_RXCSUM);
-	/* All offloads can be toggled */
-	net_dev->hw_features = net_dev->features & ~NETIF_F_HIGHDMA;
+	/* All offloads can be toggled, and so can loopback */
+	net_dev->hw_features = ((net_dev->features & ~NETIF_F_HIGHDMA) |
+				NETIF_F_LOOPBACK);
 	efx = netdev_priv(net_dev);
 	pci_set_drvdata(pci_dev, efx);
 	SET_NETDEV_DEV(net_dev, &pci_dev->dev);