netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Nicolas de Pesloüan" <nicolas.2p.debian@gmail.com>
To: Jay Vosburgh <fubar@us.ibm.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH] bonding: prevent deadlock on slave store with alb mode
Date: Wed, 25 May 2011 09:33:38 +0200	[thread overview]
Message-ID: <4DDCB0D2.2050505@gmail.com> (raw)
In-Reply-To: <20007.1306283668@death>

Le 25/05/2011 02:34, Jay Vosburgh a écrit :

> 	One alternative is to simply permit all option changes while the
> bond is down, then commit then as a set when the bond is brought up (or
> fail the open or adjust options if the options are invalid).  More or
> less what you suggest, except that the "commit" is the ifup.
>
> 	Even that is a substantial rework of how things are done now,
> since as you point out, options today take effect more or less in real
> time.

Agreed.

Setting an option while the master is down should only syntax-check the option value and store the 
value for later use (at the time we bring the bond up). Setting an option while the master is up 
should take effect in realtime if possible/appropriate or store it and issue a warning saying one 
need to down+up the bond to commit the change.

>> This also begs the question, is it or is it not safe to enslave devices while
>> the bond is down?  Clearly from the bug report its unsafe, and I don't know what
>> other (if any) conditions exist that cause problems when doing this (be that a
>> deadlock, panic or simply undefined or unexpected behavior).  If its really
>> unsafe, then issuing a warning seems incorrect, we shouldn't allow user space to
>> cause things like this, and as such, we should return an error.  If it is safe
>> (generally) and this is an isolated bug, then we should probably remove the
>> warning.  But to just issue a vague 'This might do bad things' warning seems
>> wrong in either case.
>
> 	Agreed.  I think it (conceptually) should be safe to add or
> remove slaves when the bond is down, and bonding shouldn't complain
> about it.

Are there any reasons not to consider enslavement as a normal option with the same processing as 
other options (only do real enslavement at the time we bring the master up)?

	Nicolas.

  parent reply	other threads:[~2011-05-25  7:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-24 19:36 [PATCH] bonding: prevent deadlock on slave store with alb mode Neil Horman
2011-05-24 20:00 ` Andy Gospodarek
2011-05-24 20:13   ` Nicolas de Pesloüan
2011-05-24 20:37     ` Neil Horman
2011-05-24 20:51       ` Nicolas de Pesloüan
2011-05-24 21:02         ` Nicolas de Pesloüan
2011-05-24 21:12         ` Jay Vosburgh
2011-05-24 21:14           ` David Miller
2011-05-24 21:32           ` Nicolas de Pesloüan
2011-05-24 23:18           ` Neil Horman
2011-05-25  0:34             ` Jay Vosburgh
2011-05-25  2:20               ` Neil Horman
2011-05-25  7:33               ` Nicolas de Pesloüan [this message]
2011-05-25 17:16 ` [PATCH] bonding: prevent deadlock on slave store with alb mode (v2) Neil Horman
2011-05-25 17:32   ` Jay Vosburgh
2011-05-25 21:58   ` David Miller
2011-05-25 18:13 ` [PATCH] bonding: prevent deadlock on slave store with alb mode (v3) Neil Horman
2011-05-25 18:59   ` Jay Vosburgh

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=4DDCB0D2.2050505@gmail.com \
    --to=nicolas.2p.debian@gmail.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).