netdev - Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode

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: <20160107065340.GB27753@unicorn.suse.cz>
Date:	Thu, 7 Jan 2016 07:53:40 +0100
From:	Michal Kubecek <mkubecek@...e.cz>
To:	zyjzyj2000@...il.com
Cc:	emil.s.tantilov@...el.com, jay.vosburgh@...onical.com,
	vfalico@...il.com, gospo@...ulusnetworks.com,
	netdev@...r.kernel.org, boris.shteinbock@...driver.com
Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode

On Thu, Jan 07, 2016 at 02:15:13PM +0800, zyjzyj2000@...il.com wrote:
> From: Zhu Yanjun <yanjun.zhu@...driver.com>
> 
> In 802.3ad mode, the speed and duplex is needed. But in some NIC,
> there is a time span between NIC up state and getting speed and duplex.
> As such, sometimes a slave in 802.3ad mode is in up state without
> speed and duplex. This will make bonding in 802.3ad mode can not
> work well.
> To make bonding driver be compatible with more NICs, it is
> necessary to restrict the up state in 802.3ad mode.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@...driver.com>
> ---
>  drivers/net/bonding/bond_main.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 09f8a48..7df8af5 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1991,6 +1991,17 @@ static int bond_miimon_inspect(struct bonding *bond)
>  
>  		link_state = bond_check_dev_link(bond, slave->dev, 0);
>  
> +		if ((BMSR_LSTATUS == link_state) &&
> +		    (BOND_MODE(bond) == BOND_MODE_8023AD)) {
> +			rtnl_lock();
> +			bond_update_speed_duplex(slave);
> +			rtnl_unlock();
> +			if ((slave->speed == SPEED_UNKNOWN) ||
> +			    (slave->duplex == DUPLEX_UNKNOWN)) {
> +				link_state = 0;
> +				netdev_info(bond->dev, "In 802.3ad mode, it is not enough to up without speed and duplex");

If I read this right, whenever this state (link up but speed/duplex
unknown) is entered, you'll keep writing this message into kernel log
every miimon milliseconds until something changes. I'm not sure how long
a NIC can stay in such state but it might get quite annoying (even more
if something really goes wrong and NIC stays that way which can't be
completely ruled out, IMHO).


> +			}
> +		}
>  		switch (slave->link) {
>  		case BOND_LINK_UP:
>  			if (link_state)

BtW, you accidentally submitted this patch twice.

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