On 07/03/2012 05:47 PM, Arvid Brodin :
(Added MACB "patch" contact Nicolas Ferre to CC list.)
Hi,
(adding linux-arm-kernel)
On 2012-06-29 17:24, Ben Hutchings wrote:
quoted
On Fri, 2012-06-29 at 02:36 +0000, Arvid Brodin wrote:
quoted
Hi,
After 'ip link set eth0 up' on an avr32 board (network driver macb), the device ends up in
operational mode "UNKNOWN":
# ip link
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
link/ether 00:24:74:00:17:9d brd ff:ff:ff:ff:ff:ff
Unplugging and plugging in the network cable gets the device to mode "UP".
This is a problem for me because I'm trying to use this device as a "slave" device (for a
virtual HSR device*) and I need to be able to decide if the slave device is operational or
not.
Following Stephen's advice here:
http://kerneltrap.org/mailarchive/linux-netdev/2008/9/24/3398834 I checked the macb.c code
and noticed they do not call netif_carrier_off() neither before register_netdev() nor in
dev_open().
quoted
It should be called after register_netdev() and before the driver's
ndo_open implementation returns.
After having read several drivers, it seems that some are calling
netif_carrier_off() *before* register_netdev() and some *after*... What
is the proper way?
I'm guessing this allows linkwatch to do netif_carrier_on() some time after the dev_open()?
Besides not calling netif_carrier_off() in dev_open(), the Cadence/MACB driver calls
netif_carrier_off() in dev_close(). Is this correct?
How should I handle carrier state for a virtual device? The device should have "carrier"
as long as at least one of the underlying physical interfaces is operational (which I
guess means operational state UP). Would it be correct to watch NETDEV_CHANGE and DOWN/UP
events of the slaves and call netif_carrier_on()/off() on the virtual device depending on
the slaves' states?
quoted
quoted
I added the call before register_netdev(), which fixed the problem. However, if I then
enable IPv6:
# ip link set eth0 up
ADDRCONF(NETDEV_UP): eth0: link is not ready
eth0: link up (100/Full)
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
This looks normal.
Good, that narrows it down a bit.
quoted
quoted
Any idea what is happening / what I'm doing wrong? (This is not just cosmetic; is some
situations this seems to kill the interface - e.g. ping does not work, down/up does not
help...) Things work fine without IPv6 configured.
Perhaps some packets sent automatically by IPv6 are triggering a driver
bug? Or there is a bug in multicast support, which IPv6 always uses.
Sorry, I have no clue on this topic. But I am eager to know if you find
something. I can queue your patch for netif_carrier_off() at least...
Best regards,
--
Nicolas Ferre
On Tue, 2012-07-03 at 18:15 +0200, Nicolas Ferre wrote:
On 07/03/2012 05:47 PM, Arvid Brodin :
quoted
(Added MACB "patch" contact Nicolas Ferre to CC list.)
Hi,
(adding linux-arm-kernel)
quoted
On 2012-06-29 17:24, Ben Hutchings wrote:
quoted
On Fri, 2012-06-29 at 02:36 +0000, Arvid Brodin wrote:
quoted
Hi,
After 'ip link set eth0 up' on an avr32 board (network driver macb), the device ends up in
operational mode "UNKNOWN":
# ip link
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
link/ether 00:24:74:00:17:9d brd ff:ff:ff:ff:ff:ff
Unplugging and plugging in the network cable gets the device to mode "UP".
This is a problem for me because I'm trying to use this device as a "slave" device (for a
virtual HSR device*) and I need to be able to decide if the slave device is operational or
not.
Following Stephen's advice here:
http://kerneltrap.org/mailarchive/linux-netdev/2008/9/24/3398834 I checked the macb.c code
and noticed they do not call netif_carrier_off() neither before register_netdev() nor in
dev_open().
quoted
It should be called after register_netdev() and before the driver's
ndo_open implementation returns.
After having read several drivers, it seems that some are calling
netif_carrier_off() *before* register_netdev() and some *after*... What
is the proper way?
[...]
The latter is correct; the former is a bug that is tolerated for now:
http://article.gmane.org/gmane.linux.network/111355/
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.
OFF carrier state is setup in probe() open() and suspend() functions.
The carrier ON state is managed in macb_handle_link_change().
Signed-off-by: Nicolas Ferre <redacted>
---
Hi,
This patch follows the discussion about IPv6 and macb interface
("ADDRCONF(NETDEV_UP): eth0: link is not ready" with IPv6)
It may not address all aspects of the discussion, so please do comments...
I am still wondering if netif_carrier_off() is needed in macb_probe() *after*
the call to register_netdev() in addition to its inclusion in the open()
function. As I saw it in sevral drivers, I kept it.
I have tested several suspend/resume loops, and I think that just calling
netif_carrier_off() in suspend() function is sufficient: am I right?
Thanks for your help,
Best regards,
drivers/net/ethernet/cadence/macb.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 1466bc4..033064b 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -179,13 +179,16 @@ static void macb_handle_link_change(struct net_device *dev)
spin_unlock_irqrestore(&bp->lock, flags);
if (status_change) {
- if (phydev->link)
+ if (phydev->link) {
+ netif_carrier_on(dev);
netdev_info(dev, "link up (%d/%s)\n",
phydev->speed,
phydev->duplex == DUPLEX_FULL ?
"Full" : "Half");
- else
+ } else {
+ netif_carrier_off(dev);
netdev_info(dev, "link down\n");
+ }
}
}
@@ -1033,6 +1036,9 @@ static int macb_open(struct net_device *dev)
netdev_dbg(bp->dev, "open\n");
+ /* carrier starts down */
+ netif_carrier_off(dev);
+
/* if the phy is not yet register, retry later*/
if (!bp->phy_dev)
return -EAGAIN;
@@ -1406,6 +1412,8 @@ static int __init macb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dev);
+ netif_carrier_off(dev);
+
netdev_info(dev, "Cadence %s at 0x%08lx irq %d (%pM)\n",
macb_is_gem(bp) ? "GEM" : "MACB", dev->base_addr,
dev->irq, dev->dev_addr);
@@ -1469,6 +1477,7 @@ static int macb_suspend(struct platform_device *pdev, pm_message_t state)
struct net_device *netdev = platform_get_drvdata(pdev);
struct macb *bp = netdev_priv(netdev);
+ netif_carrier_off(netdev);
netif_device_detach(netdev);
clk_disable(bp->hclk);
--
1.7.10
From: Nicolas Ferre <redacted>
Date: Wed, 4 Jul 2012 11:14:13 +0200
OFF carrier state is setup in probe() open() and suspend() functions.
The carrier ON state is managed in macb_handle_link_change().
Signed-off-by: Nicolas Ferre <redacted>
Applied to net-next, thanks.