From: "Nicolas de Pesloüan" <nicolas.2p.debian@gmail•com>
To: Jay Vosburgh <fubar@us•ibm.com>
Cc: Weiping Pan <wpan@redhat•com>, netdev@vger•kernel.org
Subject: Re: [PATCH net 2/3] bonding:check mode when modify primary_reselect
Date: Mon, 11 Jun 2012 23:13:28 +0200 [thread overview]
Message-ID: <4FD65F78.2070001@gmail.com> (raw)
In-Reply-To: <31758.1339448166@death.nxdomain>
Le 11/06/2012 22:56, Jay Vosburgh a écrit :
> Nicolas de Pesloüan <nicolas.2p.debian@gmail•com> wrote:
>
>> Le 11/06/2012 11:00, Weiping Pan a écrit :
>>> Using a primary_reselect only makes sense in active backup, TLB or ALB modes.
>>>
>>> Signed-off-by: Weiping Pan<wpan@redhat•com>
>>> ---
>>> drivers/net/bonding/bond_sysfs.c | 7 +++++++
>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>> index 485bedb..1b0f3cd 100644
>>> --- a/drivers/net/bonding/bond_sysfs.c
>>> +++ b/drivers/net/bonding/bond_sysfs.c
>>> @@ -1123,6 +1123,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
>>> if (!rtnl_trylock())
>>> return restart_syscall();
>>>
>>> + if (!USES_PRIMARY(bond->params.mode)) {
>>> + pr_err("%s: Unable to set primary_reselect; %s is in mode %d\n",
>>> + bond->dev->name, bond->dev->name, bond->params.mode);
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> new_value = bond_parse_parm(buf, pri_reselect_tbl);
>>> if (new_value< 0) {
>>> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
>>
>>
>> May I suggest we only issue a warning, store the new value for
>> primary_reselect, and avoid calling bond_select_active_slave(bond), if
>> !USE_PRIMARY(bond->params.mode)?
>>
>> That way, we do not add one more constraint on the order one must write into sysfs.
>
> I'm not in favor of changing anything here. There's already a
> message that primary_reselect is being changed, I think that's
> sufficient. The other similar cases don't issue warnings, e.g., setting
> xmit_hash_policy doesn't complain if the mode is not one that utilizes
> the hash.
Agreed. Calling bond_select_active_slave(bond) looks safe, even for mode that does not use primary,
so we don't need to change anything.
Would you support other patch similar to 1/3 in this thread, that try to relax the order to write
into sysfs for bonding?
Nicolas
next prev parent reply other threads:[~2012-06-11 21:12 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-11 9:00 [PATCH net 0/3] correct behavior when modify primary via sysfs Weiping Pan
2012-06-11 9:00 ` [PATCH net 1/3] bonding:record primary when modify it " Weiping Pan
2012-06-11 19:38 ` Nicolas de Pesloüan
2012-06-11 20:48 ` Jay Vosburgh
2012-06-12 3:38 ` Weiping Pan
2012-06-12 20:05 ` Nicolas de Pesloüan
2012-06-12 22:24 ` David Miller
2012-06-11 9:00 ` [PATCH net 2/3] bonding:check mode when modify primary_reselect Weiping Pan
2012-06-11 19:42 ` Nicolas de Pesloüan
2012-06-11 20:56 ` Jay Vosburgh
2012-06-11 21:13 ` Nicolas de Pesloüan [this message]
2012-06-11 21:28 ` Jay Vosburgh
2012-06-11 9:00 ` [PATCH net 3/3] bonding:force to use primary slave Weiping Pan
2012-06-11 19:49 ` Nicolas de Pesloüan
2012-06-11 21:17 ` Jay Vosburgh
2012-06-12 3:35 ` [PATCH net V2] " Weiping Pan
2012-06-12 5:00 ` Jay Vosburgh
2012-06-12 6:37 ` Weiping Pan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FD65F78.2070001@gmail.com \
--to=nicolas.2p.debian@gmail$(echo .)com \
--cc=fubar@us$(echo .)ibm.com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=wpan@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox