Message-Id: <C73FAA61-4C63-41FC-9BBF-C8314B9DEDE4@cumulusnetworks.com>
Date:	Sat, 9 Nov 2013 21:28:38 -1000
From:	Scott Feldman <sfeldma@...ulusnetworks.com>
To:	Jay Vosburgh <fubar@...ibm.com>
Cc:	Veaceslav Falico <vfalico@...hat.com>, andy@...yhouse.net,
	netdev@...r.kernel.org,
	Shrijeet Mukherjee <shm@...ulusnetworks.com>
Subject: Re: [PATCH net-next 3/8] bonding: add downdelay netlink support


On Nov 8, 2013, at 11:15 AM, Jay Vosburgh <fubar@...ibm.com> wrote:

> Scott Feldman <sfeldma@...ulusnetworks.com> wrote:
> 
>> What I’d like to propose, and I hope that you’ll agree, is we tackle
>> this in two phases.  The first phase is to finish the current
>> duplication effort to enable netlink equivalents of the attributes in
>> sysfs.  This is a fairly mechanical process, preserving existing
>> behavior as you point out, and keeps the patches single-minded and easy
>> to review.
>> 
>> The second phase is to revisit ordering constraints and find places
>> where we can remove constraints or streamline the dependency checks.
>> 
>> Sound like a plan?
> 
> 	Well, maybe.
> 
> 	What I want to avoid for the iproute / netlink bonding support
> is all of the hoops that the initscripts / sysconfig scripts had to jump
> through to obey the current sysfs ordering limtations.
> 
> 	The primary user of this is going to be iproute.  Will the above
> quoted limitations (and their equivalents for various other options)
> make any difference with regards to the following:
> 
> ip [...] bond arp_validate all arp_interval 1000
> ip [...] bond arp_interval 1000 arp_validate all
> 
> 	I.e., does the ordering matter at the iproute level when
> multiple options are specified simultaneously?

Surprisingly, order doesn’t matter.  Regardless of the ordering of attrs within the netlink msg, the processing order of attrs is always the same.  In this case of bonding, it’s bond_netlink.c:bond_changelink() that dictates the order attrs are processed.  So in your examples above, both versions would yield the same results.

> 	What happens if I do:
> 
> ip [...] bond arp_interval 1000 arp_validate all miimon 100
> 
> 	This is separate from simply changing one thing at a time, e.g.,
> 
> ip [...] bond arp_validate all
> ip [...] bond arp_interval 1000

This is the problem to be solved.  Bonding has several mutually exclusive attrs.  When mutually exclusive options are presented at the same time (like in you first example above), we can pick a winner using the processing order in bond_changelink().  When single attrs are set (like in second example), it’s trickier to pick winner in bond_changelink(), but do-able, I think.

> 	will presuambly hit the test above, and this is where the
> ordering stuff comes into play for sure.
> 
> 	Also, as I look at the iproute patch, it doesn't appear to
> accept the text names for the options, only numeric values (e.g., "ip
> [...]  bond arp_validate 3").  That appears to be a limitation of Jiri's
> original iproute patch as well.  Am I the only person that perfers the
> text labels (e.g., arp_validate as "all") to numbers (arp_validate as
> "3")?

I can add text names to the iproute patch for v2.

> 	So, honestly, I think if the ordering constraints are going to
> be relaxed, it should happen sooner rather than later.  Perhaps not in
> the same patch as the netlink support, but ideally at least in the same
> series, so there is no real release with the constraints.  Changing the
> constraints after the script, etc, conversion is done doesn't really
> help much.

Ok, let me study this some more before sending v2.

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

Powered by blists - more mailing lists