Ben Hutchings [off-list ref] writes:
But if you do it beforehand then it doesn't have the intended effect.
(Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)
So you have to do it after, but without dropping the RTNL lock in
between.
So you may want to add something like
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
set_bit(__LINK_STATE_NOCARRIER, &dev->state)
rtnl_unlock();
return err;
}
for these drivers?
Bjørn
quoted
Ben Hutchings [off-list ref] writes:
quoted
But if you do it beforehand then it doesn't have the intended effect.
(Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)
So you have to do it after, but without dropping the RTNL lock in
between.
So you may want to add something like
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
set_bit(__LINK_STATE_NOCARRIER, &dev->state)
rtnl_unlock();
return err;
}
for these drivers?
t looks like this variant is equivalent to the existing code:
netif_carrier_off(dev);
err = register_netdev(dev);
if (err)
goto out;
According to explanation in commit 22604c866889c4b2e12b73cbf1683bda1b72a313,
in this case "this causes these drivers to incorrectly report their
link status as IF_OPER_UNKNOWN which can falsely set the IFF_RUNNING
flag when the interface is first brought up".
As far as I understand, to fix the issue it is required to call
netif_carrier_off() itself:
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
netif_carrier_off(dev);
rtnl_unlock();
return err;
}
What do you think?
On Fri, 17 Aug 2012 11:55:43 +0400
Ilya Shchepetkov [off-list ref] wrote:
quoted
quoted
Ben Hutchings [off-list ref] writes:
quoted
But if you do it beforehand then it doesn't have the intended effect.
(Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)
So you have to do it after, but without dropping the RTNL lock in
between.
So you may want to add something like
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
set_bit(__LINK_STATE_NOCARRIER, &dev->state)
rtnl_unlock();
return err;
}
for these drivers?
t looks like this variant is equivalent to the existing code:
netif_carrier_off(dev);
err = register_netdev(dev);
if (err)
goto out;
According to explanation in commit 22604c866889c4b2e12b73cbf1683bda1b72a313,
in this case "this causes these drivers to incorrectly report their
link status as IF_OPER_UNKNOWN which can falsely set the IFF_RUNNING
flag when the interface is first brought up".
As far as I understand, to fix the issue it is required to call
netif_carrier_off() itself:
int register_netdev_carrier_off(struct net_device *dev)
{
int err;
rtnl_lock();
err = register_netdevice(dev);
if (!err)
netif_carrier_off(dev);
rtnl_unlock();
return err;
}
What do you think?
Does this prevent multiple link events from being reported to user space?
If the root cause of the problem is the link status
(commit 22604c866889c4b2e12b73cbf1683bda1b72a313), then the kernel
should be fixed to do link status correctly.
From an application point of view IFF_RUNNING is meaningless unless IFF_UP
is set.