Subject: Re: Possible bug in bonding driver (with VLAN in ALB mode)

From: Sven Anders <[email-masked]>
Date: Wed, 22 Oct 2008 16:34:45 +0200

Jay Vosburgh schrieb:
> Sven Anders <[email-masked]> wrote:
> 
>> Any other ideas?
> 
> 	Try this; I actually tested this one and it seems to not crash
> my system.  This is against the current mainline, so you might have to
> apply it by hand on your older kernel.

Ok, it did not crash, but... (see comments below)

> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 3d39278..4fb78b0 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -341,14 +341,15 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
>  
>  static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct packet_type *ptype, struct net_device *orig_dev)
>  {
> -	struct bonding *bond = bond_dev->priv;
> +	struct bonding *bond;
>  	struct arp_pkt *arp = (struct arp_pkt *)skb->data;
>  	int res = NET_RX_DROP;
>  
>  	if (dev_net(bond_dev) != &init_net)
>  		goto out;
>  
> -	if (!(bond_dev->flags & IFF_MASTER))
> +	if (!(bond_dev->priv_flags & IFF_BONDING) ||
> +	    !(bond_dev->flags & IFF_MASTER))
>  		goto out;
>  	if (!arp) {

This did not work, because the VLAN interface did not have the
BONDING flag set.

> @@ -363,6 +364,7 @@ static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct
>  
>  	if (arp->op_code == htons(ARPOP_REPLY)) {
>  		/* update rx hash table for this ARP */
> +		bond = bond_dev->priv;
>  		rlb_update_entry_from_arp(bond, arp);
>  		dprintk("Server received an ARP Reply from client\n");
>  	}

To make this work, even for the VLAN interfaces, I had to get the
stored "bond" from the "packet_type"'s private data.
Instead I used here:

     bond = ptype->af_packet_priv;
     if (!bond) goto out;

> @@ -813,7 +815,7 @@ static int rlb_initialize(struct bonding *bond)
>  
>  	/*initialize packet type*/
>  	pk_type->type = __constant_htons(ETH_P_ARP);
> -	pk_type->dev = bond->dev;
> +	pk_type->dev = NULL;
>  	pk_type->func = rlb_arp_recv;
>  
>  	/* register to receive ARPs */

Additionally, here I store the "bond" to the private data:

    pk_type->dev = NULL;
    pk_type->func = rlb_arp_recv;
    pk_type->af_packet_priv = bond;

With these fixes, it worked!
That means: Receive load balancing with VLANs works.

Certainly, this is only a test bugfix only, because it does not
work with more than one active bond. Any ideas how to implement
this correctly? Do we have to use an "dev_add_pack()" for each
VLAN we are adding?

What troubles me is, that nobody since the addition of VLAN support
to bonding in 2004 had this problem before. Did this (ever) work in
one older version of the bonding driver?

Regards
 Sven

-- 
 Sven Anders <[email-masked]>                 () Ascii Ribbon Campaign
                                                 /\ Support plain text e-mail
 ANDURAS service solutions AG
 Innstraße 71 - 94036 Passau - Germany
 Web: www.anduras.de - Tel: +49 (0)851-4 90 50-0 - Fax: +49 (0)851-4 90 50-55

Rechtsform: Aktiengesellschaft - Sitz: Passau - Amtsgericht Passau HRB 6032
Mitglieder des Vorstands: Sven Anders, Marcus Junker
Vorsitzender des Aufsichtsrats: Mark Peters