* [PATCH net V1 0/2] net/bonding: fixes related to neigh
@ 2012-04-04 8:56 Or Gerlitz
2012-04-04 8:56 ` [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release Or Gerlitz
2012-04-04 8:56 ` [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function Or Gerlitz
0 siblings, 2 replies; 7+ messages in thread
From: Or Gerlitz @ 2012-04-04 8:56 UTC (permalink / raw)
To: davem; +Cc: roland, netdev, fubar, Or Gerlitz
From: Shlomo Pongratz <shlomop@mellanox.com>
Dave,
This small patch set fixes some issues we stepped on when preparing
for some IPoIB changes.
With the 1st patch, the kernel networking code of arp_netdev_event calls
neigh_changeaddr which further flashes the boding neighbours in some more
cases which weren't covered by commit 7d26bb103 "bonding: emit event when
bonding changes MAC"
The 2nd patch fixes a bug where under bonding, the IPoIB neigh cleanup
function wasn't called.
Or.
changes from V0: applied feedback from Jay Vosburgh
- emit the address change in bond_release but not in bond_release_all
where its unneeded.
- rephrase the comment in bond_neigh_init on why we need to pre-proxy
the cleanup function. Also removed a comment which isn't needed there.
Shlomo Pongratz (2):
net/bonding: emit address change event also in bond_release
net/bonding: correctly proxy slave neigh param setup ndo function
drivers/net/bonding/bond_main.c | 54 +++++++++++++++++++++++++++++++++------
1 files changed, 46 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release
2012-04-04 8:56 [PATCH net V1 0/2] net/bonding: fixes related to neigh Or Gerlitz
@ 2012-04-04 8:56 ` Or Gerlitz
2012-04-04 16:57 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
2012-04-04 8:56 ` [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function Or Gerlitz
1 sibling, 2 replies; 7+ messages in thread
From: Or Gerlitz @ 2012-04-04 8:56 UTC (permalink / raw)
To: davem; +Cc: roland, netdev, fubar, Or Gerlitz, Shlomo Pongratz
From: Shlomo Pongratz <shlomop@mellanox.com>
commit 7d26bb103c4 "bonding: emit event when bonding changes MAC" didn't
take care to emit the NETDEV_CHANGEADDR event in bond_release, where bonding
actually changes the mac address (to all zeroes). As a result the neighbours
aren't deleted by the core networking code (which does so upon getting that
event).
Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
---
drivers/net/bonding/bond_main.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a20b585..d38f635 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2035,6 +2035,9 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
+ if (bond->slave_cnt == 0)
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
+
bond_compute_features(bond);
if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) &&
(old_features & NETIF_F_VLAN_CHALLENGED))
--
1.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function
2012-04-04 8:56 [PATCH net V1 0/2] net/bonding: fixes related to neigh Or Gerlitz
2012-04-04 8:56 ` [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release Or Gerlitz
@ 2012-04-04 8:56 ` Or Gerlitz
2012-04-04 17:00 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
1 sibling, 2 replies; 7+ messages in thread
From: Or Gerlitz @ 2012-04-04 8:56 UTC (permalink / raw)
To: davem; +Cc: roland, netdev, fubar, Or Gerlitz, Shlomo Pongratz
From: Shlomo Pongratz <shlomop@mellanox.com>
The current implemenation was buggy for slaves who use ndo_neigh_setup,
since the networking stack invokes the bonding device ndo entry (from
neigh_params_alloc) before any devices are enslaved, and the bonding
driver can't further delegate the call at that point in time. As a
result when bonding IPoIB devices, the neigh_cleanup hasn't been called.
Fix that by deferring the actual call into the slave ndo_neigh_setup
from the time the bonding neigh_setup is called.
Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
---
drivers/net/bonding/bond_main.c | 51 ++++++++++++++++++++++++++++++++------
1 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d38f635..c8dc1a5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3705,17 +3705,52 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
read_unlock(&bond->lock);
}
-static int bond_neigh_setup(struct net_device *dev, struct neigh_parms *parms)
+static int bond_neigh_init(struct neighbour *n)
{
- struct bonding *bond = netdev_priv(dev);
+ struct bonding *bond = netdev_priv(n->dev);
struct slave *slave = bond->first_slave;
+ const struct net_device_ops *slave_ops;
+ struct neigh_parms parms;
+ int ret;
+
+ if (!slave)
+ return 0;
+
+ slave_ops = slave->dev->netdev_ops;
+
+ if (!slave_ops->ndo_neigh_setup)
+ return 0;
+
+ parms.neigh_setup = NULL;
+ parms.neigh_cleanup = NULL;
+ ret = slave_ops->ndo_neigh_setup(slave->dev, &parms);
+ if (ret)
+ return ret;
+
+ /*
+ * Assign slave's neigh_cleanup to neighbour in case cleanup is called
+ * after the last slave has been detached. Assumes that all slaves
+ * utilize the same neigh_cleanup (true at this writing as only user
+ * is ipoib).
+ */
+ n->parms->neigh_cleanup = parms.neigh_cleanup;
+
+ if (!parms.neigh_setup)
+ return 0;
+
+ return parms.neigh_setup(n);
+}
+
+/*
+ * The bonding ndo_neigh_setup is called at init time beofre any
+ * slave exists. So we must declare proxy setup function which will
+ * be used at run time to resolve the actual slave neigh param setup.
+ */
+static int bond_neigh_setup(struct net_device *dev,
+ struct neigh_parms *parms)
+{
+ parms->neigh_setup = bond_neigh_init;
- if (slave) {
- const struct net_device_ops *slave_ops
- = slave->dev->netdev_ops;
- if (slave_ops->ndo_neigh_setup)
- return slave_ops->ndo_neigh_setup(slave->dev, parms);
- }
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release
2012-04-04 8:56 ` [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release Or Gerlitz
@ 2012-04-04 16:57 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Jay Vosburgh @ 2012-04-04 16:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, roland, netdev, Shlomo Pongratz
Or Gerlitz <ogerlitz@mellanox.com> wrote:
>From: Shlomo Pongratz <shlomop@mellanox.com>
>
>commit 7d26bb103c4 "bonding: emit event when bonding changes MAC" didn't
>take care to emit the NETDEV_CHANGEADDR event in bond_release, where bonding
>actually changes the mac address (to all zeroes). As a result the neighbours
>aren't deleted by the core networking code (which does so upon getting that
>event).
>
>Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>---
> drivers/net/bonding/bond_main.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index a20b585..d38f635 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2035,6 +2035,9 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
> write_unlock_bh(&bond->lock);
> unblock_netpoll_tx();
>
>+ if (bond->slave_cnt == 0)
>+ call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
>+
> bond_compute_features(bond);
> if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) &&
> (old_features & NETIF_F_VLAN_CHALLENGED))
>--
>1.7.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function
2012-04-04 8:56 ` [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function Or Gerlitz
@ 2012-04-04 17:00 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Jay Vosburgh @ 2012-04-04 17:00 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, roland, netdev, Shlomo Pongratz
Or Gerlitz <ogerlitz@mellanox.com> wrote:
>From: Shlomo Pongratz <shlomop@mellanox.com>
>
>The current implemenation was buggy for slaves who use ndo_neigh_setup,
>since the networking stack invokes the bonding device ndo entry (from
>neigh_params_alloc) before any devices are enslaved, and the bonding
>driver can't further delegate the call at that point in time. As a
>result when bonding IPoIB devices, the neigh_cleanup hasn't been called.
>
>Fix that by deferring the actual call into the slave ndo_neigh_setup
>from the time the bonding neigh_setup is called.
>
>Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>---
> drivers/net/bonding/bond_main.c | 51 ++++++++++++++++++++++++++++++++------
> 1 files changed, 43 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index d38f635..c8dc1a5 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3705,17 +3705,52 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> read_unlock(&bond->lock);
> }
>
>-static int bond_neigh_setup(struct net_device *dev, struct neigh_parms *parms)
>+static int bond_neigh_init(struct neighbour *n)
> {
>- struct bonding *bond = netdev_priv(dev);
>+ struct bonding *bond = netdev_priv(n->dev);
> struct slave *slave = bond->first_slave;
>+ const struct net_device_ops *slave_ops;
>+ struct neigh_parms parms;
>+ int ret;
>+
>+ if (!slave)
>+ return 0;
>+
>+ slave_ops = slave->dev->netdev_ops;
>+
>+ if (!slave_ops->ndo_neigh_setup)
>+ return 0;
>+
>+ parms.neigh_setup = NULL;
>+ parms.neigh_cleanup = NULL;
>+ ret = slave_ops->ndo_neigh_setup(slave->dev, &parms);
>+ if (ret)
>+ return ret;
>+
>+ /*
>+ * Assign slave's neigh_cleanup to neighbour in case cleanup is called
>+ * after the last slave has been detached. Assumes that all slaves
>+ * utilize the same neigh_cleanup (true at this writing as only user
>+ * is ipoib).
>+ */
>+ n->parms->neigh_cleanup = parms.neigh_cleanup;
>+
>+ if (!parms.neigh_setup)
>+ return 0;
>+
>+ return parms.neigh_setup(n);
>+}
>+
>+/*
>+ * The bonding ndo_neigh_setup is called at init time beofre any
>+ * slave exists. So we must declare proxy setup function which will
>+ * be used at run time to resolve the actual slave neigh param setup.
>+ */
>+static int bond_neigh_setup(struct net_device *dev,
>+ struct neigh_parms *parms)
>+{
>+ parms->neigh_setup = bond_neigh_init;
>
>- if (slave) {
>- const struct net_device_ops *slave_ops
>- = slave->dev->netdev_ops;
>- if (slave_ops->ndo_neigh_setup)
>- return slave_ops->ndo_neigh_setup(slave->dev, parms);
>- }
> return 0;
> }
>
>--
>1.7.1
>
>--
>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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release
2012-04-04 8:56 ` [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release Or Gerlitz
2012-04-04 16:57 ` Jay Vosburgh
@ 2012-04-04 22:12 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2012-04-04 22:12 UTC (permalink / raw)
To: ogerlitz; +Cc: roland, netdev, fubar, shlomop
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 4 Apr 2012 11:56:19 +0300
> From: Shlomo Pongratz <shlomop@mellanox.com>
>
> commit 7d26bb103c4 "bonding: emit event when bonding changes MAC" didn't
> take care to emit the NETDEV_CHANGEADDR event in bond_release, where bonding
> actually changes the mac address (to all zeroes). As a result the neighbours
> aren't deleted by the core networking code (which does so upon getting that
> event).
>
> Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function
2012-04-04 8:56 ` [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function Or Gerlitz
2012-04-04 17:00 ` Jay Vosburgh
@ 2012-04-04 22:12 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2012-04-04 22:12 UTC (permalink / raw)
To: ogerlitz; +Cc: roland, netdev, fubar, shlomop
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 4 Apr 2012 11:56:20 +0300
> From: Shlomo Pongratz <shlomop@mellanox.com>
>
> The current implemenation was buggy for slaves who use ndo_neigh_setup,
> since the networking stack invokes the bonding device ndo entry (from
> neigh_params_alloc) before any devices are enslaved, and the bonding
> driver can't further delegate the call at that point in time. As a
> result when bonding IPoIB devices, the neigh_cleanup hasn't been called.
>
> Fix that by deferring the actual call into the slave ndo_neigh_setup
> from the time the bonding neigh_setup is called.
>
> Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-04-04 22:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-04 8:56 [PATCH net V1 0/2] net/bonding: fixes related to neigh Or Gerlitz
2012-04-04 8:56 ` [PATCH net V1 1/2] net/bonding: emit address change event also in bond_release Or Gerlitz
2012-04-04 16:57 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
2012-04-04 8:56 ` [PATCH net V1 2/2] net/bonding: correctly proxy slave neigh param setup ndo function Or Gerlitz
2012-04-04 17:00 ` Jay Vosburgh
2012-04-04 22:12 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.