| 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
| ||
|
Message-ID: <20140117065718.GA5699@redhat.com>
Date: Fri, 17 Jan 2014 07:57:18 +0100
From: Veaceslav Falico <vfalico@...hat.com>
To: Jay Vosburgh <fubar@...ibm.com>
Cc: netdev@...r.kernel.org, Andy Gospodarek <andy@...yhouse.net>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next 0/6] bonding: only rely on arp packets if arp
monitor is used
On Thu, Jan 16, 2014 at 02:38:59PM -0800, Jay Vosburgh wrote:
...snip...
> I think the bottom line here is pretty simple:
>
> Using the ARP monitor with the loadbalance modes is not a common
>configuration in my experience, and making it work is tricky. However,
>anyone using it today will be relying on the current behavior, which we
>therefore must not change.
Yep, agreed. It might be against the documentation, these use cases might
be weird/illogical - but they (kind of) work, and we both agree that this
change might break them, so it's definitely a no go.
OTOH, I'd still like to help people who have some kind of broadcast traffic
(STP, CDP, some routing etc.) running over network and keeping their slaves
up (and those that cannot/don't want to use arp_validate=3).
What do you think about this*? It's on top of this series, extends
arp_validate to (not) filter out ARPs on not-validated slaves and permits
it to be used in non-AB mode (also, we don't need that bond->lock, we're
always under RCU).
*:
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index a4d925e..7223ef4 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).
For the active slave, the validation checks ARP replies to
confirm that they were generated by an arp_ip_target. Since
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0f613ae..2ef1d5a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -246,6 +246,9 @@ const struct bond_parm_tbl arp_validate_tbl[] = {
{ "active", BOND_ARP_VALIDATE_ACTIVE},
{ "backup", BOND_ARP_VALIDATE_BACKUP},
{ "all", BOND_ARP_VALIDATE_ALL},
+{ "arp", BOND_ARP_VALIDATE_ARP},
+{ "active_arp", BOND_ARP_VALIDATE_ACTIVE_ARP},
+{ "backup_arp", BOND_ARP_VALIDATE_BACKUP_ARP},
{ NULL, -1},
};
@@ -2284,16 +2287,15 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
struct arphdr *arp = (struct arphdr *)skb->data;
unsigned char *arp_ptr;
__be32 sip, tip;
- int alen;
-
- if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
- return RX_HANDLER_ANOTHER;
-
- read_lock(&bond->lock);
+ int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
if (!slave_do_arp_validate(bond, slave)) {
- slave->last_arp_rx = jiffies;
- goto out_unlock;
+ if ((slave_do_arp_validate_only(bond, slave) && is_arp) ||
+ !slave_do_arp_validate_only(bond, slave))
+ slave->last_arp_rx = jiffies;
+ return RX_HANDLER_ANOTHER;
+ } else if (!is_arp) {
+ return RX_HANDLER_ANOTHER;
}
alen = arp_hdr_len(bond->dev);
@@ -2349,7 +2351,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
bond_validate_arp(bond, slave, tip, sip);
out_unlock:
- read_unlock(&bond->lock);
if (arp != (struct arphdr *)skb->data)
kfree(arp);
return RX_HANDLER_ANOTHER;
@@ -4181,10 +4182,6 @@ static int bond_check_params(struct bond_params *params)
}
if (arp_validate) {
- if (bond_mode != BOND_MODE_ACTIVEBACKUP) {
- pr_err("arp_validate only supported in active-backup mode\n");
- return -EINVAL;
- }
if (!arp_interval) {
pr_err("arp_validate requires arp_interval\n");
return -EINVAL;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bab20e..9d6d231 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -434,12 +434,6 @@ int bond_option_arp_validate_set(struct bonding *bond, int arp_validate)
return -EINVAL;
}
- if (bond->params.mode != BOND_MODE_ACTIVEBACKUP) {
- pr_err("%s: arp_validate only supported in active-backup mode.\n",
- bond->dev->name);
- return -EINVAL;
- }
-
pr_info("%s: setting arp_validate to %s (%d).\n",
bond->dev->name, arp_validate_tbl[arp_validate].modename,
arp_validate);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9f07af1..19eb023 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -319,6 +319,11 @@ static inline bool bond_is_active_slave(struct slave *slave)
#define BOND_ARP_VALIDATE_BACKUP (1 << BOND_STATE_BACKUP)
#define BOND_ARP_VALIDATE_ALL (BOND_ARP_VALIDATE_ACTIVE | \
BOND_ARP_VALIDATE_BACKUP)
+#define BOND_ARP_VALIDATE_ARP (BOND_ARP_VALIDATE_ALL + 1) /* бля... */
+#define BOND_ARP_VALIDATE_ACTIVE_ARP (BOND_ARP_VALIDATE_ACTIVE | \
+ BOND_ARP_VALIDATE_ARP)
+#define BOND_ARP_VALIDATE_BACKUP_ARP (BOND_ARP_VALIDATE_BACKUP | \
+ BOND_ARP_VALIDATE_ARP)
static inline int slave_do_arp_validate(struct bonding *bond,
struct slave *slave)
@@ -326,6 +331,12 @@ static inline int slave_do_arp_validate(struct bonding *bond,
return bond->params.arp_validate & (1 << bond_slave_state(slave));
}
+static inline int slave_do_arp_validate_only(struct bonding *bond,
+ struct slave *slave)
+{
+ return bond->params.arp_validate & BOND_ARP_VALIDATE_ARP;
+}
+
/* Get the oldest arp which we've received on this slave for bond's
* arp_targets.
*/
>
> -J
--
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