Re: 答复: Re: [Patch] netxen_nic: Bugfix for wrong RUNNING status of NX3031 NICs with no link

On Fri, 2014-01-03 at 15:07 +0800, jiang.biao2@zte.com.cn wrote:

quoted

quoted

quoted

You cannot do this, because at the very exact moment you call
register_netdevice() the device can be brought up and once the
device is up the link can be brought up.

quoted

quoted

quoted

Therefore if you invoke netif_carrier_off() right after
register_netdevice(), it can cancel out a legitimate
netif_carrier_on() call.

quoted

quoted

register_netdevice() should be call in probe(), at that time, is
it impossible to bring up device because the driver loading has
not finished?

quoted

So far as the networking core is concerned, when register_netdevice() is
called the device is ready to use.  But you can do:

quoted

rtnl_lock();
register_netdevice();
netif_carrier_off();
rtnl_unlock();

quoted

However, as I said before, the current order in netxen_nic works and
only older kernel versions require you to call netif_carrier_off() after
registering the device.

quoted

quoted

Even that, the netif_carrier_off() will be called before
netif_carrier_on(), and that will not be a problem.

quoted

quoted

Besides, the other net drivers(e.g igb, e1000, e1000e) I saw all
call netif_carrier_off() after register_netdev(). There is no such
problems in these drivers.

quoted

It is not easy to hit the race condition but it is real.

But the problem I depicted really exists in the current netxen_nic driver,
you can easily reproduce it following the reproduction steps.

'Current' as in Linux 3.12/3.13-rc7?

I don't know where I would find a Netxen card now.  So I can't test
netxen_nic myself, but I can assure you the sfc driver does things in
the same order and it works correctly.

Can the following modification fix both the problem I faced and the race
you worried?

netif_carrier_off();
netdev->operstate=IF_OPER_DOWN;
register_netdevice();

register_netdevice() calls linkwatch_init_dev() which sets the operstate
if necessary.  You should not set it directly.

Ben.

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