netdev - Re: [PATCH] bonding: document two undocumented options.

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3571.1312405193@death>
Date:	Wed, 03 Aug 2011 13:59:53 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	=?ISO-8859-1?Q?Nicolas_de_Peslo=FCan?= 
	<nicolas.2p.debian@...il.com>
cc:	David Miller <davem@...emloft.net>,
	Stephen Hemminger <shemminger@...tta.com>,
	nicolas.2p.debian@...e.fr, andy@...yhouse.net,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] bonding: document two undocumented options.

Nicolas de Pesloüan <nicolas.2p.debian@...il.com> wrote:

>Le 03/08/2011 12:44, David Miller a écrit :
>> From: Nicolas de Pesloüan<nicolas.2p.debian@...e.fr>
>> Date: Tue,  2 Aug 2011 22:06:55 +0200
>>
>>> Commit 655f8919d549ad1872e24d826b6ce42530516d2e
>>>      bonding: add min links parameter to 802.3ad
>>>
>>> and commit ebd8e4977a87cb81d93c62a9bff0102a9713722f
>>>      bonding: add all_slaves_active parameter
>>>
>>> introduced new options to bonding, but didn't provide the documentation
>>> for those options.
>>>
>>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@...e.fr>
>>
>> Please explicitly mention in each new entry what the default
>> setting is.
>
>Unfortunately, I failed to find a place in the bonding code where the
>max_links option is initialized with a default value. So I must assume
>default value is zero which should cause carrier to always be asserted, or
>undefined, which should cause interesting side effects...
>
>The obvious default value should be 1, but I cannot confirm it is.

	Looking at it now, I see no initialization, and it's a static,
so I believe it will end up being zero.  From the code, zero seems like
the proper default, since it will make this test never pass:

                /* are enough slaves available to consider link up? */
                if (active->num_of_ports < bond->params.min_links) {
                        if (netif_carrier_ok(bond->dev)) {
                                netif_carrier_off(bond->dev);
                                return 1;
                        }

	This will cause carrier to be asserted (for 802.3ad mode)
whenever there is an active aggregator, regardless of the number of
available links in that aggregator.

>Stephen, as the author of this feature, can you please clarify what the default value for min_links is?
>
>V2 will follow, giving the real default value for all_slaves_active and
>what I consider the sensible default value for max_links, even if the
>technical real default value is currently unclear.

	I think the actual and sensible default are both zero, although
the documentation should probably mention that a value of zero is magic
and won't ever set the bond down due to too few ports (links) active.

	Or, perhaps describe it how it actually works: if there are
fewer than "min_links" ports in the active aggregator, the bond is set
carrier down.  The default min_links value of zero means that the bond
will never be set down due to having too few ports active.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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