netdev - Re: [PATCH 1/1] net: Set NOCARRIER bit of etherdev state at initialization

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: <1400866681.18322.27.camel@dcbw.local>
Date:	Fri, 23 May 2014 12:38:01 -0500
From:	Dan Williams <dcbw@...hat.com>
To:	Balakumaran Kannan <kumaran.4353@...il.com>
Cc:	davem@...emloft.net, stephen@...workplumber.org,
	avi.kp.137@...il.com, edumazet@...gle.com, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net: Set NOCARRIER bit of etherdev state at
 initialization

On Fri, 2014-05-23 at 22:31 +0530, Balakumaran Kannan wrote:
> For any etherdev 'netif_carrier_ok' should succeed only after phy
> layer determines carrier state through auto-negotiation or setting
> it forcibly.
> 
> So ethernet drivers should set '__LINK_STATE_NOCARRIER' bit of
> 'dev->state' using 'netif_carrier_off' call at driver initialization.
> As it is common for all ethernet devices, 'netif_carrier_off' call
> could be moved to 'ether_setup'.

Unless I misunderstand, wouldn't this turn off carrier for all devices
until the driver explicitly sets the carrier up?

The current "carrier on until told otherwise" model is intentional,
because not all drivers support carrier detection, and thus we must
assume the carrier is on until the driver tells us it is not on.

Would this patch break that model?

Dan

> Signed-off-by: Balakumaran Kannan <kumaran.4353@...il.com>
> ---
> 'netif_carrier_off' could be called anywhere in between
> 'alloc_etherdev' and 'phy_start'. For example,
> 1. e1000 driver calls at the end of 'e1000_probe'
> 2. intel i40e driver calls at the beginning of 'i40e_open'
> 3. realtek r8169 driver calls at the end of 'rtl_init_one'
> 
> But some driver misses this call. For example smsc911x driver doesn't
> call 'netif_carrier_off' before calling 'phy_start'. So device starts
> performing dhcp and IPv6 DAD even if carrier is not ready.
> 
> So it is better to move 'netif_carrier_off' call to 'ether_setup'. And
> drivers can remove this call from their initialization functions.
> ---
>  net/ethernet/eth.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 5dc638c..45f6703 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -369,6 +369,9 @@ void ether_setup(struct net_device *dev)
>  
>  	memset(dev->broadcast, 0xFF, ETH_ALEN);
>  
> +	/* Carrier state should be off initially */
> +	netif_carrier_off(dev);
> +
>  }
>  EXPORT_SYMBOL(ether_setup);
>  


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