netdev - Re: [PATCH net-next 3/8] bonding: add downdelay netlink support

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]

Date:	Thu, 7 Nov 2013 13:59:56 -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 7, 2013, at 7:05 AM, Jay Vosburgh <fubar@...ibm.com> wrote:

> Scott Feldman <sfeldma@...ulusnetworks.com> wrote:
> 
>> +int bond_option_downdelay_set(struct bonding *bond, int downdelay)
>> +{
>> +	if (!(bond->params.miimon)) {
>> +		pr_err("%s: Unable to set down delay as MII monitoring is disabled\n", bond->dev->name);
>> +		return -EPERM;
>> +	}
> 
> 	I would argue that it is better to permit this option to be set
> at any time, regardless of whether miimon is enabled or not.  This
> removes an ordering constraint and makes things generally easier to deal
> with.  Setting the option has no effect if miimon is not set, but that's
> ok.
> 
> 	My comment here would also apply to other options that are
> "secondary," in the sense that they affect the behavior of other options
> or modes, e.g., updelay, arp_validate (in another path of this series),
> arp_ip_targets, etc.
> 
> 	I'm aware that this is simply duplicating the existing behavior
> of the sysfs API, but I'd be in favor of changing that, too.  These
> limitations are a holdover from the module paramters days, and should
> have been made more flexible a long time ago.

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?

-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