[PATCH 7/9] [ETH]: Start net device with carrier down

[PATCH 7/9] [ETH]: Start net device with carrier down

[Trent Piepho]

Because it _is_ down when the device starts.

The device's carrier status is controlled via the functions
netif_carrier_on() and netif_carrier_off().  These set or clear a bit
indicating the lower level link aka carrier is down, and if the state
changed, they fire off a routing netlink event.

The problem is that when the device is first created and opened, the state
bit indicating the carrier is down isn't set, i.e. the state is wrong.
When the carrier comes up for the first time no netlink event is sent,
since the device state indicated the carrier was already up.

Signed-off-by: Trent Piepho <tpiepho@freescale.com>
CC: Andy Fleming <afleming@freescale.com>
---
 net/ethernet/eth.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index a80839b..cdba6d0 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -333,6 +333,7 @@ void ether_setup(struct net_device *dev)
 	dev->addr_len		= ETH_ALEN;
 	dev->tx_queue_len	= 1000;	/* Ethernet wants good queues */
 	dev->flags		= IFF_BROADCAST|IFF_MULTICAST;
+	dev->state		= 1 << __LINK_STATE_NOCARRIER;
 
 	memset(dev->broadcast, 0xFF, ETH_ALEN);
 
-- 
1.5.4.1

Re: [PATCH 7/9] [ETH]: Start net device with carrier down

[Stephen Hemminger]

On Wed, 24 Sep 2008 14:20:55 -0700
Trent Piepho <tpiepho@freescale.com> wrote:

> Because it _is_ down when the device starts.
> 
> The device's carrier status is controlled via the functions
> netif_carrier_on() and netif_carrier_off().  These set or clear a bit
> indicating the lower level link aka carrier is down, and if the state
> changed, they fire off a routing netlink event.
> 
> The problem is that when the device is first created and opened, the state
> bit indicating the carrier is down isn't set, i.e. the state is wrong.
> When the carrier comes up for the first time no netlink event is sent,
> since the device state indicated the carrier was already up.
> 
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> CC: Andy Fleming <afleming@freescale.com>
> ---
>  net/ethernet/eth.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index a80839b..cdba6d0 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -333,6 +333,7 @@ void ether_setup(struct net_device *dev)
>  	dev->addr_len		= ETH_ALEN;
>  	dev->tx_queue_len	= 1000;	/* Ethernet wants good queues */
>  	dev->flags		= IFF_BROADCAST|IFF_MULTICAST;
> +	dev->state		= 1 << __LINK_STATE_NOCARRIER;
>  
>  	memset(dev->broadcast, 0xFF, ETH_ALEN);
>  

This breaks devices that never call netif_carrier_on.

Standard practice is to call netif_carrier_off after allocation but before
register_netdev

Re: [PATCH 7/9] [ETH]: Start net device with carrier down

[David Miller]

From: Trent Piepho <tpiepho@freescale.com>
Date: Wed, 24 Sep 2008 14:20:55 -0700

> The problem is that when the device is first created and opened, the state
> bit indicating the carrier is down isn't set, i.e. the state is wrong.
> When the carrier comes up for the first time no netlink event is sent,
> since the device state indicated the carrier was already up.

It works properly if the device driver does as it is supposed to,
which is invoke netif_carrier_off() in it's ->open() method in this
situation.

Please fix the drivers where this is not happening.

Re: [PATCH 7/9] [ETH]: Start net device with carrier down

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 24 Sep 2008 14:32:03 -0700

> This breaks devices that never call netif_carrier_on.
> 
> Standard practice is to call netif_carrier_off after allocation but before
> register_netdev

Or in ->open()

Re: [PATCH 7/9] [ETH]: Start net device with carrier down

[Trent Piepho]

On Wed, 24 Sep 2008, David Miller wrote:
> From: Trent Piepho <tpiepho@freescale.com>
> Date: Wed, 24 Sep 2008 14:20:55 -0700
>
>> The problem is that when the device is first created and opened, the state
>> bit indicating the carrier is down isn't set, i.e. the state is wrong.
>> When the carrier comes up for the first time no netlink event is sent,
>> since the device state indicated the carrier was already up.
>
> It works properly if the device driver does as it is supposed to,
> which is invoke netif_carrier_off() in it's ->open() method in this
> situation.
>
> Please fix the drivers where this is not happening.

I tried that first, but if the carrier is already up, opening the interface
generated extra rt-netlink messages for carrier down then carrier up, even
though the carrier was up all along.

But maybe I didn't have it at the right place in the open() method.  I'll try
Stephen's suggestion to put it after allocation but before register_netdev()
and see if the problem is still there.

[PATCH v2] gianfar: Create net device with carrier down

[Trent Piepho]

The device's carrier status is controlled via the functions
netif_carrier_on() and netif_carrier_off().  These set or clear a bit
indicating the carrier (aka lower level link) is down, and if the state
changed, they fire off a routing netlink event.

Add a call to netif_carrier_off() before register_netdev() so that the
newly created device will be set to carrier down.  Then when the carrier
comes up for the first time, a netlink event will be generated, as the
carrier changed from down to up.  Otherwise the initial carrier up will
appear to be changing the status from up to up, and so no event is
generated since that's not a change.

Signed-off-by: Trent Piepho <tpiepho@freescale.com>
CC: Andy Fleming <afleming@freescale.com>
---
This uses Stephen's suggestion, which works fine.  I had already tried this
in ->open(), but that creates bogus carrier up->carrier down->carrier up
netlink events when the device is opened.

 drivers/net/gianfar.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index a671270..484cf69 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -339,6 +339,9 @@ static int gfar_probe(struct platform_device *pdev)
 	/* Enable most messages by default */
 	priv->msg_enable = (NETIF_MSG_IFUP << 1 ) - 1;
 
+	/* Carrier starts down, phylib will bring it up */
+	netif_carrier_off(dev);
+
 	err = register_netdev(dev);
 
 	if (err) {
-- 
1.5.4.1