netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Veaceslav Falico <vfalico@redhat.com>
To: Jay Vosburgh <fubar@us.ibm.com>
Cc: netdev@vger.kernel.org, Andy Gospodarek <andy@greyhouse.net>
Subject: Re: [PATCH v2 net-next 06/12] bonding: document the new _arp options for arp_validate
Date: Mon, 17 Feb 2014 14:36:07 +0100	[thread overview]
Message-ID: <20140217133607.GB13038@redhat.com> (raw)
In-Reply-To: <32051.1389991108@death.nxdomain>

On Fri, Jan 17, 2014 at 12:38:28PM -0800, Jay Vosburgh wrote:
>Veaceslav Falico <vfalico@redhat.com> wrote:
>
>>CC: Jay Vosburgh <fubar@us.ibm.com>
>>CC: Andy Gospodarek <andy@greyhouse.net>
>>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>>---
>> Documentation/networking/bonding.txt | 34 ++++++++++++++++++++++++++++++----
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>>index 3620690..a0c1bca2 100644
>>--- a/Documentation/networking/bonding.txt
>>+++ b/Documentation/networking/bonding.txt
>>@@ -279,19 +279,45 @@ arp_validate
>>
>> 	none or 0
>>
>>-		No validation is performed.  This is the default.
>>+		No validation is performed.  This is the default. Any arriving
>>+		traffic (arp or non-arp) is considered a proof that the slave
>>+		is up.
>>
>> 	active or 1
>>
>>-		Validation is performed only for the active slave.
>>+		Validation is performed only for the active slave. Only ARPs
>>+		that arrive from any arp_ip_target are considered legit. The
>>+		backup slave still does no validation (as if arp_validate=0).
>>
>> 	backup or 2
>>
>>-		Validation is performed only for backup slaves.
>>+		Validation is performed only for backup slaves. Only ARPs
>>+		that arrive from any arp_ip_target are considered legit. The
>>+		active slave still has no validation (as if arp_validate=0).
>>
>> 	all or 3
>>
>>-		Validation is performed for all slaves.
>>+		Validation is performed for all slaves. Only ARPs
>>+		that arrive from any arp_ip_target are considered legit.
>>+
>>+	arp or 4
>>+
>>+		Any arp packet is accepted as a proof that any slave is up,
>>+		but no IP-based validation is made.
>>+
>>+	active_arp or 5
>>+
>>+		Validation is performed only for the active slave. Only ARPs
>>+		that arrive from any arp_ip_target are considered legit. The
>>+		backup slave validates only arp packets, but doesn't check the
>>+		source (as if arp_validate=4).
>>+
>>+	backup_any or 6
>>+
>>+		Validation is performed only for backup slaves. Only ARPs
>>+		that arrive from any arp_ip_target are considered legit. The
>>+		active slave validates only arp packets, but doesn't check the
>>+		source (as if arp_validate=4).

Hi Jay,

Sorry for taking it so long - however I didn't manage to update/re-send the
patchset before the net-next closure, so sending it now. Few comments
below, though.

>
>	I think that, for the last three options, saying that
>"validation is performed" is not quite right, since the next paragraph
>goes on to explain what "validation" is (that the incoming ARP came from
>us or was a response to ours), and these options don't really validate
>in that sense, but merely filter anything that's not an ARP.

Yes and no. Indeed, validation is not performed if we use the "filter_all"
option (filter both slaves, don't do any validation). However it's
performed for both filter_active/backup - as in "Validate arp packets on
active/backup, and filter only on the other one".

However your text is really awesome in explaining what is
validating/filtering, so I've included it in the patch with minor
corrections, see below.

>
>	There'a a sentence with a similar problem further down: "Use of
>the arp_validate option can resolve this, as the ARP monitor will only
>consider ARP requests and replies associated with its own instance of
>bonding."  For the three new options, this sentence is not accurate.
>
>	I think I'd rework this whole block something like the following
>(this is a diff against your patched version).  I'm calling the two
>separate things "validation" and "filtering," since the wording you used
>kind of combined things into two styles of validation; I think it's
>clearer to make them discrete things.
>
>	This would also necessitate change the option tag names; I also
>put the "filter" ones into the same order as the "validate" ones
>(active, backup, then all).

I've left the original order, because the "filter" is, actually, bit 4, so
if we want to filter AND validate, we set FITLER | VALIDATE_{ACTIVE,BACKUP},
and this way we get 5 or 6. I've also changed the filter_all to just
"filter", to make it more clear what's happening. Here are the only lines
changed on top of your patch:

         filter or 4

                 Filtering is applied to all slaves. No validation is
                 performed.

         filter_active or 5

                 Filtering is applied to all slaves, validation is performed
                 only for the active slave.

         filter_backup or 6

                 Filtering is applied to all slaves, validation is performed
                 only for backup slaves.

This way we can set any pair of filtering/validation:

No validation + no filtering? none
Validation + no filtering? acitve/backup
No validation + filtering? filter
Validation + filtering? filter_active/backup

I'll send v3 in a few minutes, so that it'll be easier to review (it's also
rebased on top of latest net-next).

Thanks a lot, and sorry for the delay.

>
>	Comments?
>
>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>index a0c1bca2..5fd6a6a 100644
>--- a/Documentation/networking/bonding.txt
>+++ b/Documentation/networking/bonding.txt
>@@ -270,80 +270,87 @@ arp_ip_target
> arp_validate
>
> 	Specifies whether or not ARP probes and replies should be
>-	validated in any mode that supports arp monitoring.  This causes
>-	the ARP monitor to examine the incoming ARP requests and replies,
>-	and only consider a slave to be up if it is receiving the
>-	appropriate ARP traffic.
>-
>+	validated in any mode that supports arp monitoring, or whether
>+	non-ARP traffic should be filtered (disregarded) for link
>+	monitoring purposes.
>+	
> 	Possible values are:
>
> 	none or 0
>
>-		No validation is performed.  This is the default. Any arriving
>-		traffic (arp or non-arp) is considered a proof that the slave
>-		is up.
>+		No validation or filtering is performed.
>
> 	active or 1
>
>-		Validation is performed only for the active slave. Only ARPs
>-		that arrive from any arp_ip_target are considered legit. The
>-		backup slave still does no validation (as if arp_validate=0).
>+		Validation is performed only for the active slave.
>
> 	backup or 2
>
>-		Validation is performed only for backup slaves. Only ARPs
>-		that arrive from any arp_ip_target are considered legit. The
>-		active slave still has no validation (as if arp_validate=0).
>+		Validation is performed only for backup slaves.
>
> 	all or 3
>
>-		Validation is performed for all slaves. Only ARPs
>-		that arrive from any arp_ip_target are considered legit.
>-
>-	arp or 4
>-
>-		Any arp packet is accepted as a proof that any slave is up,
>-		but no IP-based validation is made.
>-
>-	active_arp or 5
>-
>-		Validation is performed only for the active slave. Only ARPs
>-		that arrive from any arp_ip_target are considered legit. The
>-		backup slave validates only arp packets, but doesn't check the
>-		source (as if arp_validate=4).
>-
>-	backup_any or 6
>-
>-		Validation is performed only for backup slaves. Only ARPs
>-		that arrive from any arp_ip_target are considered legit. The
>-		active slave validates only arp packets, but doesn't check the
>-		source (as if arp_validate=4).
>-
>-	For the active slave, the validation checks ARP replies to
>-	confirm that they were generated by an arp_ip_target.  Since
>-	backup slaves do not typically receive these replies, the
>-	validation performed for backup slaves is on the ARP request
>-	sent out via the active slave.  It is possible that some
>-	switch or network configurations may result in situations
>-	wherein the backup slaves do not receive the ARP requests; in
>-	such a situation, validation of backup slaves must be
>-	disabled.
>-
>-	The validation of ARP requests on backup slaves is mainly
>-	helping bonding to decide which slaves are more likely to
>-	work in case of the active slave failure, it doesn't really
>-	guarantee that the backup slave will work if it's selected
>-	as the next active slave.
>-
>-	This option is useful in network configurations in which
>-	multiple bonding hosts are concurrently issuing ARPs to one or
>-	more targets beyond a common switch.  Should the link between
>-	the switch and target fail (but not the switch itself), the
>-	probe traffic generated by the multiple bonding instances will
>-	fool the standard ARP monitor into considering the links as
>-	still up.  Use of the arp_validate option can resolve this, as
>-	the ARP monitor will only consider ARP requests and replies
>-	associated with its own instance of bonding.
>+		Validation is performed for all slaves.
>+
>+	filter_active or 4
>+
>+		Filtering is applied to the active slave only.
>+
>+	filter_backup or 5
>+
>+		Filtering is applied to the backup slave(s) only.
>+
>+	filter_all or 6
>+
>+		Filtering is applied to all slaves.
>+
>+	Validation:
>+
>+	Enabling validation causes the ARP monitor to examine the incoming
>+	ARP requests and replies, and only consider a slave to be up if it
>+	is receiving the appropriate ARP traffic.
>+
>+	For an active slave, the validation checks ARP replies to confirm
>+	that they were generated by an arp_ip_target.  Since backup slaves
>+	do not typically receive these replies, the validation performed
>+	for backup slaves is on the broadcast ARP request sent out via the
>+	active slave.  It is possible that some switch or network
>+	configurations may result in situations wherein the backup slaves
>+	do not receive the ARP requests; in such a situation, validation
>+	of backup slaves must be disabled.
>+
>+	The validation of ARP requests on backup slaves is mainly helping
>+	bonding to decide which slaves are more likely to work in case of
>+	the active slave failure, it doesn't really guarantee that the
>+	backup slave will work if it's selected as the next active slave.
>+
>+	Validation is useful in network configurations in which multiple
>+	bonding hosts are concurrently issuing ARPs to one or more targets
>+	beyond a common switch.  Should the link between the switch and
>+	target fail (but not the switch itself), the probe traffic
>+	generated by the multiple bonding instances will fool the standard
>+	ARP monitor into considering the links as still up.  Use of
>+	validation can resolve this, as the ARP monitor will only consider
>+	ARP requests and replies associated with its own instance of
>+	bonding.
>+
>+	Filtering:
>+
>+	Enabling filtering causes the ARP monitor to only use incoming ARP
>+	packets for link availability purposes.  Arriving packets that are
>+	not ARPs are delivered normally, but do not count when determining
>+	if a slave is available.
>+
>+	Filtering operates by only considering the reception of ARP
>+	packets (any ARP packet, regardless of source or destination) when
>+	determining if a slave has received traffic for link availability
>+	purposes.
>+
>+	Filtering is useful in network configurations in which significant
>+	levels of third party broadcast traffic would fool the standard
>+	ARP monitor into considering the links as still up.  Use of
>+	filtering can resolve this, as only ARP traffic is considered for
>+	link availability purposes.
>
> 	This option was added in bonding version 3.1.0.
>
>
>
>---
>	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>

  reply	other threads:[~2014-02-17 13:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-17 16:58 [PATCH v2 net-next 0/12] bonding: add an option to rely on unvalidated arp packets Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 01/12] bonding: remove bond->lock from bond_arp_rcv Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 02/12] bonding: permit using arp_validate with non-ab modes Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 03/12] bonding: always update last_arp_rx on packet recieve Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 04/12] bonding: always set recv_probe to bond_arp_rcv in arp monitor Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 05/12] bonding: extend arp_validate to be able to receive unvalidated arp-only traffic Veaceslav Falico
2014-01-17 20:41   ` Jay Vosburgh
2014-01-23 10:25     ` Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 06/12] bonding: document the new _arp options for arp_validate Veaceslav Falico
2014-01-17 20:38   ` Jay Vosburgh
2014-02-17 13:36     ` Veaceslav Falico [this message]
2014-01-17 16:58 ` [PATCH v2 net-next 07/12] bonding: use the new options to correctly set last_arp_rx Veaceslav Falico
2014-01-17 20:32   ` Neil Horman
2014-01-17 16:58 ` [PATCH v2 net-next 08/12] bonding: use last_arp_rx in slave_last_rx() Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 09/12] bonding: use last_arp_rx in bond_loadbalance_arp_mon() Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 10/12] bonding: remove useless updating of slave->dev->last_rx Veaceslav Falico
2014-01-17 16:58 ` [PATCH v2 net-next 11/12] bonding: trivial: rename slave->jiffies to ->last_link_up Veaceslav Falico
2014-01-17 16:59 ` [PATCH v2 net-next 12/12] bonding: rename last_arp_rx to last_rx Veaceslav Falico

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=20140217133607.GB13038@redhat.com \
    --to=vfalico@redhat.com \
    --cc=andy@greyhouse.net \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    /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).