bridging + load balancing bonding

bridging + load balancing bonding

[Jasper Spaans]

Hi,

We're using the following setup for bonding and bridging, to be able to put
large amounts of data through multiple IDS analyzers:

                             +---[br0]----+     +--- eth1 ---(IDS machine 1)
(Span port from switch) -- eth0          bond0--+
                                                +--- eth2 ---(IDS machine 2)

eth0 receives network traffic, which should be passed to machines which are
connected to eth1 and eth2. These machines run an IDS package, and there are
two of those for performance reasons.

bond0 is configured to load balance the packets using "balance-xor", in this
case combined with xmit_hash_policy layer2.

However, we're seeing problems: packets from one flow do not end up at the
same IDS machine.  This is because this selection is not based on the source
_and_ destination mac addresses of the original packet, but on the mac
address of the bonding device and the destination mac address of the
package.

This is also clear in the code:
For example, in bond_main.c, in bond_xmit_hash_policy_l2:
	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;

Changing this to
	return (data->h_dest[5] ^ data->h_source[5]) % count;
fixes our problems, but is this harmful for packets originating locally (or
being routed?)

If not, can this be applied? Or does anyone have other ideas?

Thanks,
Jasper Spaans
-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: bridging + load balancing bonding

[Eric Dumazet]

Jasper Spaans a écrit :
> Hi,
> 
> We're using the following setup for bonding and bridging, to be able to put
> large amounts of data through multiple IDS analyzers:
> 
>                              +---[br0]----+     +--- eth1 ---(IDS machine 1)
> (Span port from switch) -- eth0          bond0--+
>                                                 +--- eth2 ---(IDS machine 2)
> 
> eth0 receives network traffic, which should be passed to machines which are
> connected to eth1 and eth2. These machines run an IDS package, and there are
> two of those for performance reasons.
> 
> bond0 is configured to load balance the packets using "balance-xor", in this
> case combined with xmit_hash_policy layer2.
> 
> However, we're seeing problems: packets from one flow do not end up at the
> same IDS machine.  This is because this selection is not based on the source
> _and_ destination mac addresses of the original packet, but on the mac
> address of the bonding device and the destination mac address of the
> package.
> 
> This is also clear in the code:
> For example, in bond_main.c, in bond_xmit_hash_policy_l2:
> 	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
> 
> Changing this to
> 	return (data->h_dest[5] ^ data->h_source[5]) % count;
> fixes our problems, but is this harmful for packets originating locally (or
> being routed?)
> 
> If not, can this be applied? Or does anyone have other ideas?
> 

Hi Jasper

Very nice setup, and nice finding.

Dont locally generated (or outed) packets have h_source set to bond_dev->dev_addr anyway ?

So your solution might be the right fix...

About other ideas... I was thinking of TEE target (not in mainline unfortunatly) :

iptables -t mangle -A PREROUTING -i eth0 <some hash on mac addr>  -j TEE --gateway 192.168.99.1  # IDS1
iptables -t mangle -A PREROUTING -i eth0 !<some hash on mac addr>  -j TEE --gateway 192.168.99.2  # IDS2

Re: bridging + load balancing bonding

[Jay Vosburgh]

Eric Dumazet <eric.dumazet@gmail.com> wrote:

>Jasper Spaans a écrit :
>> Hi,
>> 
>> We're using the following setup for bonding and bridging, to be able to put
>> large amounts of data through multiple IDS analyzers:
>> 
>>                              +---[br0]----+     +--- eth1 ---(IDS machine 1)
>> (Span port from switch) -- eth0          bond0--+
>>                                                 +--- eth2 ---(IDS machine 2)
>> 
>> eth0 receives network traffic, which should be passed to machines which are
>> connected to eth1 and eth2. These machines run an IDS package, and there are
>> two of those for performance reasons.
>> 
>> bond0 is configured to load balance the packets using "balance-xor", in this
>> case combined with xmit_hash_policy layer2.
>> 
>> However, we're seeing problems: packets from one flow do not end up at the
>> same IDS machine.  This is because this selection is not based on the source
>> _and_ destination mac addresses of the original packet, but on the mac
>> address of the bonding device and the destination mac address of the
>> package.

	By "packets from one flow" do you really mean that packets from
a given "flow" (TCP connection, UDP "stream", etc) are not always
delivered to the same bonding port?  I.e., that two packets from the
same "flow" will be delivered to different ports?  I'm not sure how
that's possible unless the source MAC in the packets changes during the
course of the flow.

	Or is your problem really that the balance algorithm on the
bonding send side doesn't match the algorithm used on the other side of
the IDS machines coming the other direction (and, thus, packets for a
given flow going in one direction end up at a different IDS than the
packets going the other direction)?

>> This is also clear in the code:
>> For example, in bond_main.c, in bond_xmit_hash_policy_l2:
>> 	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
>> 
>> Changing this to
>> 	return (data->h_dest[5] ^ data->h_source[5]) % count;
>> fixes our problems, but is this harmful for packets originating locally (or
>> being routed?)
>> 
>> If not, can this be applied? Or does anyone have other ideas?
>> 
>
>Hi Jasper
>
>Very nice setup, and nice finding.
>
>Dont locally generated (or outed) packets have h_source set to
>bond_dev->dev_addr anyway ?

	Locally generated packets do, but he's got a bridge in there, so
the traffic they're balancing is presumably not locally generated (i.e.,
is being forwarded by the bridge, in which case they'll still bear the
source MAC of the originating node on the subnet).  If the packets were
being routed instead of bridged, then, yah, they'd have the bond's
source MAC.

>So your solution might be the right fix...

	Yes, I think he's found a legitimate bug, one that only will
manifest when balancing bridged traffic.  I had to think for a minute if
this change would break anything, and I'm coming up empty.  Locally
generated or routed traffic won't see a change, and bridged traffic will
be correctly balanced according to the "source MAC XOR destination MAC"
forumla described in the documentation.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: bridging + load balancing bonding

[Eric Dumazet]

Jay Vosburgh a écrit :

> 	By "packets from one flow" do you really mean that packets from
> a given "flow" (TCP connection, UDP "stream", etc) are not always
> delivered to the same bonding port?  I.e., that two packets from the
> same "flow" will be delivered to different ports?  I'm not sure how
> that's possible unless the source MAC in the packets changes during the
> course of the flow.
> 
> 	Or is your problem really that the balance algorithm on the
> bonding send side doesn't match the algorithm used on the other side of
> the IDS machines coming the other direction (and, thus, packets for a
> given flow going in one direction end up at a different IDS than the
> packets going the other direction)?
> 

Yes this is probably Jasper problem : catch both direction on same IDS target

Say you have machine A with MAC address MAC_A
and machine B with MAC address MAC_B
(I suspect asymetric routing on A or B is out of the question :) )

A tcp / udp/ whatever protocol flow is running between these two machines

When machine A sends a frame to machine B, Jasper machine
receives a copy of this frame, with eth->src = MAC_A and eth->dst = MAC_B

With current xor algo, we perform a hash on (bond->dev_addr[5] ^ MAC_B[5])  -> IDS X

When machine B sends a frame to machine A, Jasper machine
receives a copy of this frame, with eth->src = MAC_B and eth->dst = MAC_A

With current xor algo, we peform a hash on (bond->dev_addr[5] ^ MAC_A[5])  -> possibly other IDS Y


With his fix, algo is a commutative hash (MAC_A[5] ^ MAC_B[5]) ==  (MAC_B[5] ^ MAC_A[5])



I suspect multicast/broadcast trafic should be sent to both IDS, so bonding might be inappropriate anyway...

an iptables solution might be more powerfull

Re: bridging + load balancing bonding

[Jasper Spaans]

On Thu, Oct 22, 2009 at 07:36:00PM +0200, Jay Vosburgh wrote:
> 	By "packets from one flow" do you really mean that packets from
> a given "flow" (TCP connection, UDP "stream", etc) are not always
> delivered to the same bonding port?  I.e., that two packets from the
> same "flow" will be delivered to different ports?  I'm not sure how
> that's possible unless the source MAC in the packets changes during the
> course of the flow.
> 
> 	Or is your problem really that the balance algorithm on the
> bonding send side doesn't match the algorithm used on the other side of
> the IDS machines coming the other direction (and, thus, packets for a
> given flow going in one direction end up at a different IDS than the
> packets going the other direction)?

It's the second problem: traffic in one direction ends up at another
node than traffic in the other direction, even if between the same pair of
machines.

> 	Locally generated packets do, but he's got a bridge in there, so
> the traffic they're balancing is presumably not locally generated (i.e.,
> is being forwarded by the bridge, in which case they'll still bear the
> source MAC of the originating node on the subnet).  If the packets were
> being routed instead of bridged, then, yah, they'd have the bond's
> source MAC.

And in case of routing, the destination MAC will also be modified.. so worst
case (all traffic goes to one node), no balancing will happen. This also
affects non-IDS use of load balancing, I guess.

> >So your solution might be the right fix...
> 
> 	Yes, I think he's found a legitimate bug, one that only will
> manifest when balancing bridged traffic.  I had to think for a minute if
> this change would break anything, and I'm coming up empty.  Locally
> generated or routed traffic won't see a change, and bridged traffic will
> be correctly balanced according to the "source MAC XOR destination MAC"
> forumla described in the documentation.

I'll post a patch shortly.

Jasper
-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

[PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[Jasper Spaans]

Modify bonding hash transmit policies to use the psource MAC address of
the packet instead of the MAC address configured for the bonding device.

The old sitation conflicts with the documentation.
---
 drivers/net/bonding/bond_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 69c5b15..b140b52 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3668,7 +3668,7 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
 			(data->h_dest[5] ^ bond_dev->dev_addr[5])) % count;
 	}
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*
@@ -3695,7 +3695,7 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
 
 	}
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*
@@ -3706,7 +3706,7 @@ static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*-------------------------- Device entry points ----------------------------*/
-- 
1.6.0.4


-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: [PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[Eric Dumazet]

Jasper Spaans a écrit :
> Modify bonding hash transmit policies to use the psource MAC address of
> the packet instead of the MAC address configured for the bonding device.
> 
> The old sitation conflicts with the documentation.
> ---
>  drivers/net/bonding/bond_main.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 69c5b15..b140b52 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3668,7 +3668,7 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
>  			(data->h_dest[5] ^ bond_dev->dev_addr[5])) % count;

You forgot one bond_dev->dev_addr[5] occurrence here


>  	}
>  
> -	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
> +	return (data->h_dest[5] ^ data->h_source[5]) % count;
>  }

Could you check if bond->xmit_hash_policy(skb, bond_dev, bond->slave_cnt);
could be replaced by bond->xmit_hash_policy(skb, bond->slave_cnt),
now that various implementations dont need bond_dev anymore ?

static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count) ...

Dont forget your "Signed-off-by: Jasper Spaans <spaans@fox-it.com>",
copied to "David S. Miller" <davem@davemloft.net>  and "Jay Vosburgh" <fubar@us.ibm.com>
(respectively network and bonding maintainers)

Thanks

[PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[Jasper Spaans]

Modify bonding hash transmit policies to use the psource MAC address of
the packet instead of the MAC address configured for the bonding device.

The old sitation conflicts with the documentation.

Signed-off-by: Jasper Spaans <spaans@fox-it.com>
---
 drivers/net/bonding/bond_main.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 69c5b15..3f05267 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3665,10 +3665,10 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
 
 	if (skb->protocol == htons(ETH_P_IP)) {
 		return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
-			(data->h_dest[5] ^ bond_dev->dev_addr[5])) % count;
+			(data->h_dest[5] ^ data->h_source[5])) % count;
 	}
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*
@@ -3695,7 +3695,7 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
 
 	}
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*
@@ -3706,7 +3706,7 @@ static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 
-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
+	return (data->h_dest[5] ^ data->h_source[5]) % count;
 }
 
 /*-------------------------- Device entry points ----------------------------*/
-- 
1.6.0.4


-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: [PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[Eric Dumazet]

Jasper Spaans a écrit :
> Modify bonding hash transmit policies to use the psource MAC address of
> the packet instead of the MAC address configured for the bonding device.
> 
> The old sitation conflicts with the documentation.
> 
> Signed-off-by: Jasper Spaans <spaans@fox-it.com>

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[Jay Vosburgh]

Jasper Spaans <spaans@fox-it.com> wrote:

>Modify bonding hash transmit policies to use the psource MAC address of
>the packet instead of the MAC address configured for the bonding device.
>
>The old sitation conflicts with the documentation.
>
>Signed-off-by: Jasper Spaans <spaans@fox-it.com>

	Looks good.

	Dave, please apply.  This may be suitable for -stable, as it's
pretty much a day one bug.

	-J

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

>---
> drivers/net/bonding/bond_main.c |    8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 69c5b15..3f05267 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3665,10 +3665,10 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
>
> 	if (skb->protocol == htons(ETH_P_IP)) {
> 		return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
>-			(data->h_dest[5] ^ bond_dev->dev_addr[5])) % count;
>+			(data->h_dest[5] ^ data->h_source[5])) % count;
> 	}
>
>-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
>+	return (data->h_dest[5] ^ data->h_source[5]) % count;
> }
>
> /*
>@@ -3695,7 +3695,7 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
>
> 	}
>
>-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
>+	return (data->h_dest[5] ^ data->h_source[5]) % count;
> }
>
> /*
>@@ -3706,7 +3706,7 @@ static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
>
>-	return (data->h_dest[5] ^ bond_dev->dev_addr[5]) % count;
>+	return (data->h_dest[5] ^ data->h_source[5]) % count;
> }
>
> /*-------------------------- Device entry points ----------------------------*/
>-- 
>1.6.0.4
>
>
>-- 
>Fox-IT Experts in IT Security!
>T: +31 (0) 15 284 79 99
>KvK Haaglanden 27301624
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Modify bonding hash transmit policies to use the packet's source MAC address

[David Miller]

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Fri, 23 Oct 2009 09:23:12 -0700

> Jasper Spaans <spaans@fox-it.com> wrote:
> 
>>Modify bonding hash transmit policies to use the psource MAC address of
>>the packet instead of the MAC address configured for the bonding device.
>>
>>The old sitation conflicts with the documentation.
>>
>>Signed-off-by: Jasper Spaans <spaans@fox-it.com>
> 
> 	Looks good.
> 
> 	Dave, please apply.  This may be suitable for -stable, as it's
> pretty much a day one bug.

Applied, and queued up for stable, thanks everyone.

[PATCH] Remove bond_dev from xmit_hash_policy call.

[Jasper Spaans]

Now that the bonding device is no longer used in determining the device to
which to send packets, it can be dropped from the argument list of the various
xmit_hash_policy calls.

Signed-off-by: Jasper Spaans <spaans@fox-it.com>
---
 drivers/net/bonding/bond_3ad.c  |   11 +++++------
 drivers/net/bonding/bond_main.c |   11 ++++-------
 drivers/net/bonding/bonding.h   |    3 +--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c3fa31c..3cd8153 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1956,7 +1956,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	struct port *port, *prev_port, *temp_port;
 	struct aggregator *aggregator, *new_aggregator, *temp_aggregator;
 	int select_new_active_agg = 0;
-	
+
 	// find the aggregator related to this slave
 	aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
@@ -2024,7 +2024,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 
 				// clear the aggregator
 				ad_clear_agg(aggregator);
-				
+
 				if (select_new_active_agg) {
 					ad_agg_selection_logic(__get_first_agg(port));
 				}
@@ -2075,7 +2075,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			}
 		}
 	}
-	port->slave=NULL;	
+	port->slave=NULL;
 }
 
 /**
@@ -2301,7 +2301,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 }
 
 /*
- * set link state for bonding master: if we have an active 
+ * set link state for bonding master: if we have an active
  * aggregator, we're up, if not, we're down.  Presumes that we cannot
  * have an active aggregator if there are no slaves with link up.
  *
@@ -2395,7 +2395,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	slave_agg_no = bond->xmit_hash_policy(skb, dev, slaves_in_agg);
+	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
 
 	bond_for_each_slave(bond, slave, i) {
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
@@ -2468,4 +2468,3 @@ out:
 
 	return ret;
 }
-
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3f05267..13058c5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3657,8 +3657,7 @@ void bond_unregister_arp(struct bonding *bond)
  * Hash for the output device based upon layer 2 and layer 3 data. If
  * the packet is not IP mimic bond_xmit_hash_policy_l2()
  */
-static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
-				     struct net_device *bond_dev, int count)
+static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 	struct iphdr *iph = ip_hdr(skb);
@@ -3676,8 +3675,7 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
  * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
  * altogether not IP, mimic bond_xmit_hash_policy_l2()
  */
-static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
-				    struct net_device *bond_dev, int count)
+static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 	struct iphdr *iph = ip_hdr(skb);
@@ -3701,8 +3699,7 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
 /*
  * Hash for the output device based upon layer 2 data
  */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
-				   struct net_device *bond_dev, int count)
+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 
@@ -4295,7 +4292,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 	if (!BOND_IS_OK(bond))
 		goto out;
 
-	slave_no = bond->xmit_hash_policy(skb, bond_dev, bond->slave_cnt);
+	slave_no = bond->xmit_hash_policy(skb, bond->slave_cnt);
 
 	bond_for_each_slave(bond, slave, i) {
 		slave_no--;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 6824771..02e1f9e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -204,7 +204,7 @@ struct bonding {
 #endif /* CONFIG_PROC_FS */
 	struct   list_head bond_list;
 	struct   dev_mc_list *mc_list;
-	int      (*xmit_hash_policy)(struct sk_buff *, struct net_device *, int);
+	int      (*xmit_hash_policy)(struct sk_buff *, int);
 	__be32   master_ip;
 	u16      flags;
 	u16      rr_tx_counter;
@@ -370,4 +370,3 @@ static inline void bond_unregister_ipv6_notifier(void)
 #endif
 
 #endif /* _LINUX_BONDING_H */
-
-- 
1.6.0.4


-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: [PATCH] Remove bond_dev from xmit_hash_policy call.

[Eric Dumazet]

Jasper Spaans a écrit :
> Now that the bonding device is no longer used in determining the device to
> which to send packets, it can be dropped from the argument list of the various
> xmit_hash_policy calls.
> 
> Signed-off-by: Jasper Spaans <spaans@fox-it.com>

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Next step would be to use reciprocal divide :)
I'll take care of this eventually.

Re: [PATCH] Remove bond_dev from xmit_hash_policy call.

[Jay Vosburgh]

Jasper Spaans <spaans@fox-it.com> wrote:

>Now that the bonding device is no longer used in determining the device to
>which to send packets, it can be dropped from the argument list of the various
>xmit_hash_policy calls.
>
>Signed-off-by: Jasper Spaans <spaans@fox-it.com>

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>


>---
> drivers/net/bonding/bond_3ad.c  |   11 +++++------
> drivers/net/bonding/bond_main.c |   11 ++++-------
> drivers/net/bonding/bonding.h   |    3 +--
> 3 files changed, 10 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c3fa31c..3cd8153 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -1956,7 +1956,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
> 	struct port *port, *prev_port, *temp_port;
> 	struct aggregator *aggregator, *new_aggregator, *temp_aggregator;
> 	int select_new_active_agg = 0;
>-	
>+
> 	// find the aggregator related to this slave
> 	aggregator = &(SLAVE_AD_INFO(slave).aggregator);
>
>@@ -2024,7 +2024,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
>
> 				// clear the aggregator
> 				ad_clear_agg(aggregator);
>-				
>+
> 				if (select_new_active_agg) {
> 					ad_agg_selection_logic(__get_first_agg(port));
> 				}
>@@ -2075,7 +2075,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
> 			}
> 		}
> 	}
>-	port->slave=NULL;	
>+	port->slave=NULL;
> }
>
> /**
>@@ -2301,7 +2301,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
> }
>
> /*
>- * set link state for bonding master: if we have an active 
>+ * set link state for bonding master: if we have an active
>  * aggregator, we're up, if not, we're down.  Presumes that we cannot
>  * have an active aggregator if there are no slaves with link up.
>  *
>@@ -2395,7 +2395,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
> 		goto out;
> 	}
>
>-	slave_agg_no = bond->xmit_hash_policy(skb, dev, slaves_in_agg);
>+	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
>
> 	bond_for_each_slave(bond, slave, i) {
> 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
>@@ -2468,4 +2468,3 @@ out:
>
> 	return ret;
> }
>-
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 3f05267..13058c5 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3657,8 +3657,7 @@ void bond_unregister_arp(struct bonding *bond)
>  * Hash for the output device based upon layer 2 and layer 3 data. If
>  * the packet is not IP mimic bond_xmit_hash_policy_l2()
>  */
>-static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
>-				     struct net_device *bond_dev, int count)
>+static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
> 	struct iphdr *iph = ip_hdr(skb);
>@@ -3676,8 +3675,7 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb,
>  * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
>  * altogether not IP, mimic bond_xmit_hash_policy_l2()
>  */
>-static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
>-				    struct net_device *bond_dev, int count)
>+static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
> 	struct iphdr *iph = ip_hdr(skb);
>@@ -3701,8 +3699,7 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb,
> /*
>  * Hash for the output device based upon layer 2 data
>  */
>-static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
>-				   struct net_device *bond_dev, int count)
>+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
>
>@@ -4295,7 +4292,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
> 	if (!BOND_IS_OK(bond))
> 		goto out;
>
>-	slave_no = bond->xmit_hash_policy(skb, bond_dev, bond->slave_cnt);
>+	slave_no = bond->xmit_hash_policy(skb, bond->slave_cnt);
>
> 	bond_for_each_slave(bond, slave, i) {
> 		slave_no--;
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 6824771..02e1f9e 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -204,7 +204,7 @@ struct bonding {
> #endif /* CONFIG_PROC_FS */
> 	struct   list_head bond_list;
> 	struct   dev_mc_list *mc_list;
>-	int      (*xmit_hash_policy)(struct sk_buff *, struct net_device *, int);
>+	int      (*xmit_hash_policy)(struct sk_buff *, int);
> 	__be32   master_ip;
> 	u16      flags;
> 	u16      rr_tx_counter;
>@@ -370,4 +370,3 @@ static inline void bond_unregister_ipv6_notifier(void)
> #endif
>
> #endif /* _LINUX_BONDING_H */
>-
>-- 
>1.6.0.4
>
>
>-- 
>Fox-IT Experts in IT Security!
>T: +31 (0) 15 284 79 99
>KvK Haaglanden 27301624
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove bond_dev from xmit_hash_policy call.

[David Miller]

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Fri, 23 Oct 2009 09:24:31 -0700

> Jasper Spaans <spaans@fox-it.com> wrote:
> 
>>Now that the bonding device is no longer used in determining the device to
>>which to send packets, it can be dropped from the argument list of the various
>>xmit_hash_policy calls.
>>
>>Signed-off-by: Jasper Spaans <spaans@fox-it.com>
> 
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

Applied, thanks everyone.

Re: bridging + load balancing bonding

[Jasper Spaans]

Hi Eric,

On Thu, Oct 22, 2009 at 05:41:48PM +0200, Eric Dumazet wrote:

> Very nice setup, and nice finding.
> 
> Dont locally generated (or outed) packets have h_source set to bond_dev->dev_addr anyway ?
> 
> So your solution might be the right fix...
> 
> About other ideas... I was thinking of TEE target (not in mainline unfortunatly) :
> 
> iptables -t mangle -A PREROUTING -i eth0 <some hash on mac addr>  -j TEE --gateway 192.168.99.1  # IDS1
> iptables -t mangle -A PREROUTING -i eth0 !<some hash on mac addr>  -j TEE --gateway 192.168.99.2  # IDS2

Unfortunately, this won't work: the TEE target works at IP-level, and
changes mac-addresses, which is a no-go thing for us.. (and we won't be able
to see non-IP traffic such as ARP on the IDS machines)

Jasper
-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: bridging + load balancing bonding

[Eric Dumazet]

Jasper Spaans a écrit :
> Hi Eric,
> 
> On Thu, Oct 22, 2009 at 05:41:48PM +0200, Eric Dumazet wrote:
> 
>> Very nice setup, and nice finding.
>>
>> Dont locally generated (or outed) packets have h_source set to bond_dev->dev_addr anyway ?
>>
>> So your solution might be the right fix...
>>
>> About other ideas... I was thinking of TEE target (not in mainline unfortunatly) :
>>
>> iptables -t mangle -A PREROUTING -i eth0 <some hash on mac addr>  -j TEE --gateway 192.168.99.1  # IDS1
>> iptables -t mangle -A PREROUTING -i eth0 !<some hash on mac addr>  -j TEE --gateway 192.168.99.2  # IDS2
> 
> Unfortunately, this won't work: the TEE target works at IP-level, and
> changes mac-addresses, which is a no-go thing for us.. (and we won't be able
> to see non-IP traffic such as ARP on the IDS machines)
> 

Of course, iptables / TEE works at IP level, so you'll need some ebtables analogy to work at ethernet level.

Dont you think special attention is needed for multicast/broadcast trafic (they should be sent to both IDS) ?

Re: bridging + load balancing bonding

[Jasper Spaans]

On Fri, Oct 23, 2009 at 10:55:31AM +0200, Eric Dumazet wrote:

> Dont you think special attention is needed for multicast/broadcast trafic
> (they should be sent to both IDS) ?

Not really -- AFAIK we're currently not using the information encapsulated
in broadcast traffic to judge the unicast packets. Besides, we're
aggregating the results from the IDSes, so it shouldn't matter on which node
a packet is processed.

But if the need arises, this could be added quite easily to the code.

Jasper
-- 
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624

Re: bridging + load balancing bonding

[Eric Dumazet]

Jasper Spaans a écrit :
> On Fri, Oct 23, 2009 at 10:55:31AM +0200, Eric Dumazet wrote:
> 
>> Dont you think special attention is needed for multicast/broadcast trafic
>> (they should be sent to both IDS) ?
> 
> Not really -- AFAIK we're currently not using the information encapsulated
> in broadcast traffic to judge the unicast packets. Besides, we're
> aggregating the results from the IDSes, so it shouldn't matter on which node
> a packet is processed.
> 
> But if the need arises, this could be added quite easily to the code.
> 

OK

I found following very interesting article that can give pretty good advices

http://lwn.net/Articles/145406/