Subject: Re: [PATCHv3 2/2] tg3: Allow ethtool to enable/disable loopback.

From: Stephen Hemminger <[email-masked]>
Date: Wed, 4 May 2011 08:04:08 -0700

On Wed, 4 May 2011 13:11:12 +0200
Michał Mirosław <[email-masked]> wrote:

> On Tue, May 03, 2011 at 06:18:55PM -0700, Mahesh Bandewar wrote:
> > This patch adds tg3_set_features() to handle loopback mode. Currently the
> > capability is added for the devices which support internal MAC loopback mode.
> > So when enabled, it enables internal-MAC loopback.
> [...]
> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> > index 7c7c9a8..46de633 100644
> > --- a/drivers/net/tg3.c
> > +++ b/drivers/net/tg3.c
> > @@ -6319,6 +6319,51 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
> >  	return features;
> >  }
> >  
> > +static int tg3_set_features(struct net_device *dev, u32 features)
> > +{
> > +	struct tg3 *tp = netdev_priv(dev);
> > +	u32 cur_mode = 0;
> > +	int err = 0;
> > +
> > +	if (!netif_running(dev)) {
> > +		err = -EAGAIN;
> > +		goto sfeatures_out;
> > +	}
> 
> netdev_update_features() is not designed to handle -EAGAIN from
> ndo_set_features callback. It might be useful to implement this
> handling, but in this case you should just return 0 and check
> dev->features in ndo_open callback.
> 
> Best Regards,
> Michał Mirosław

EAGAIN is a bad choice of error code anyway. It implies that the
application should retry, which in this case is not true.

This error code is used for things like flow controlled sockets where
the application should do a select/poll and the condition will clear
when other side has read.

Why not use ENETDOWN instead?