[ofa-general] Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver

Michael S. Tsirkin mst at dev.mellanox.co.il
Tue Jul 31 07:48:27 PDT 2007


> Quoting Or Gerlitz <ogerlitz at voltaire.com>:
> Subject: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for?the?bonding driver
> 
> Michael S. Tsirkin wrote:
> >>Quoting Or Gerlitz <ogerlitz at voltaire.com>:
> 
> >>To be precise, bonding will copy all the symbols it copies today from 
> >>the slave module (ipoib), see bond_setup_by_slave() in patch 3/7
> 
> >Not really.
> >This copying of symbols is something that you added, isn't it?
> >So with this approach, it won't be needed.
> 
> >It's always wrong to copy symbols from another module without
> >referencing it.
> 
> Its the --first-- time you make this comment,

It's really a well known fact. That's where the crash
with modprobe -r comes from, right?

> please suggest a different approach,

I don't know, really - if you want to access a module, you really must get
a reference to it, or to the device.
How about adding the module pointer to struct net_device?

>the relevant code is below.

>+static void bond_setup_by_slave(struct net_device *bond_dev,
>+				struct net_device *slave_dev)
>+{
>+	bond_dev->hard_header	        = slave_dev->hard_header;
>+	bond_dev->rebuild_header        = slave_dev->rebuild_header;
>+	bond_dev->hard_header_cache	= slave_dev->hard_header_cache;
>+	bond_dev->header_cache_update   = slave_dev->header_cache_update;
>+	bond_dev->hard_header_parse	= slave_dev->hard_header_parse;
>+
>+	bond_dev->neigh_setup           = slave_dev->neigh_setup;
>+
>+	bond_dev->type		    = slave_dev->type;
>+	bond_dev->hard_header_len   = slave_dev->hard_header_len;
>+	bond_dev->addr_len	    = slave_dev->addr_len;
>+
>+	memcpy(bond_dev->broadcast, slave_dev->broadcast,
>+		slave_dev->addr_len);
>+}
>+

Hmm, it seems that switching to hard_header_cache as I suggested won't help at all.
I wonder: is bonding currently broken with devices that implement
hard_header_cache/header_cache_update?

-- 
MST



More information about the general mailing list