Subject: Re: [PATCH] bonding: document two undocumented options.

From: Nicolas de Pesloüan<[email-masked]>
Date: Thu, 04 Aug 2011 07:41:53 +0200

Le 03/08/2011 22:59, Jay Vosburgh a écrit :
> Nicolas de Pesloüan<[email-masked]>  wrote:

<snip>

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

Well, you are right, but for as far as I understand, using 1 as the default value for min_links 
would cause the exact same behavior.

And 1 would be less "magical" and as such, more understandable from a user point of view. User might 
understand min_links=0 as "assert carrier on if at least 0 link have carrier on", which might be 
understood as "always assert carrier on".

	Nicolas.