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

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: <20110504080408.0142f701@nehalam>
Date:	Wed, 4 May 2011 08:04:08 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:	Mahesh Bandewar <maheshb@...gle.com>,
	Matt Carlson <mcarlson@...adcom.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Michael Chan <mchan@...adcom.com>,
	Ben Hutchings <bhutchings@...arflare.com>,
	Tom Herbert <therbert@...gle.com>
Subject: Re: [PATCHv3 2/2] tg3: Allow ethtool to enable/disable loopback.

On Wed, 4 May 2011 13:11:12 +0200
Michał Mirosław <mirq-linux@...e.qmqm.pl> 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?
--
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