From f27eff1ffd65236b8e421188f76ad1b0b94e06eb Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:47:27 +1000 Subject: [PATCH 1/4] [PATCH] iseries_veth: Don't send packets to LPARs which aren't up Hi Andrew, Jeff, The iseries_veth driver has a logic bug which means it will erroneously send packets to LPARs for which we don't have a connection. This usually isn't a big problem because the Hypervisor call fails gracefully and we return, but if packets are TX'ed during the negotiation of the connection bad things might happen. Regardless, the right thing is to bail early if we know there's no connection. Signed-off-by: Michael Ellerman --- drivers/net/iseries_veth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c index 855f8b2cf13b..7d0ef2969b4e 100644 --- a/drivers/net/iseries_veth.c +++ b/drivers/net/iseries_veth.c @@ -924,7 +924,7 @@ static int veth_transmit_to_one(struct sk_buff *skb, HvLpIndex rlp, spin_lock_irqsave(&cnx->lock, flags); - if (! cnx->state & VETH_STATE_READY) + if (! (cnx->state & VETH_STATE_READY)) goto drop; if ((skb->len - 14) > VETH_MAX_MTU) From eb235aef724568ae15af831968000cf9a3974b26 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:53:18 +1000 Subject: [PATCH 2/4] [PATCH] iseries_veth: Set dev->trans_start so watchdog timer works right Hi Andrew, Jeff, The iseries_veth driver doesn't set dev->trans_start in it's TX path. This will cause the net device watchdog timer to fire earlier than we want it to, which causes the driver to needlessly reset its connections to other LPARs. Signed-off-by: Michael Ellerman --- drivers/net/iseries_veth.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c index 7d0ef2969b4e..1e869df656c1 100644 --- a/drivers/net/iseries_veth.c +++ b/drivers/net/iseries_veth.c @@ -1023,6 +1023,8 @@ static int veth_start_xmit(struct sk_buff *skb, struct net_device *dev) lpmask = veth_transmit_to_many(skb, lpmask, dev); + dev->trans_start = jiffies; + if (! lpmask) { dev_kfree_skb(skb); } else { From 41664c03f6c96a1f8a91714309b36f1b5ca85610 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:55:08 +1000 Subject: [PATCH 3/4] [PATCH] iseries_veth: Don't leak skbs in RX path Hi Andrew, Jeff, Under some strange circumstances the iseries_veth driver can leak skbs. Fix is simply to call dev_kfree_skb() in the right place. Fix up the comment as well. Signed-off-by: Michael Ellerman --- drivers/net/iseries_veth.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c index 1e869df656c1..1edecb10993d 100644 --- a/drivers/net/iseries_veth.c +++ b/drivers/net/iseries_veth.c @@ -1264,13 +1264,18 @@ static void veth_receive(struct veth_lpar_connection *cnx, vlan = skb->data[9]; dev = veth_dev[vlan]; - if (! dev) - /* Some earlier versions of the driver sent - broadcasts down all connections, even to - lpars that weren't on the relevant vlan. - So ignore packets belonging to a vlan we're - not on. */ + if (! dev) { + /* + * Some earlier versions of the driver sent + * broadcasts down all connections, even to lpars + * that weren't on the relevant vlan. So ignore + * packets belonging to a vlan we're not on. + * We can also be here if we receive packets while + * the driver is going down, because then dev is NULL. + */ + dev_kfree_skb_irq(skb); continue; + } port = (struct veth_port *)dev->priv; dest = *((u64 *) skb->data) & 0xFFFFFFFFFFFF0000; From b2e0852e1eee7c445b1789bef41204b64f981102 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 18:09:45 +1000 Subject: [PATCH 4/4] [PATCH] iseries_veth: Cleanup skbs to prevent unregister_netdevice() hanging Hi Andrew, Jeff, The iseries_veth driver is badly behaved in that it will keep TX packets hanging around forever if they're not ACK'ed and the queue never fills up. This causes the unregister_netdevice code to wait forever when we try to take the device down, because there's still skbs around with references to our struct net_device. There's already code to cleanup any un-ACK'ed packets in veth_stop_connection() but it's being called after we unregister the net_device, which is too late. The fix is to rearrange the module exit function so that we cleanup any outstanding skbs and then unregister the driver. Signed-off-by: Michael Ellerman --- drivers/net/iseries_veth.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c index 1edecb10993d..13ed8dc1e91d 100644 --- a/drivers/net/iseries_veth.c +++ b/drivers/net/iseries_veth.c @@ -1388,18 +1388,25 @@ void __exit veth_module_cleanup(void) { int i; - vio_unregister_driver(&veth_driver); + /* Stop the queues first to stop any new packets being sent. */ + for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++) + if (veth_dev[i]) + netif_stop_queue(veth_dev[i]); + /* Stop the connections before we unregister the driver. This + * ensures there's no skbs lying around holding the device open. */ for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) veth_stop_connection(i); HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan); /* Hypervisor callbacks may have scheduled more work while we - * were destroying connections. Now that we've disconnected from + * were stoping connections. Now that we've disconnected from * the hypervisor make sure everything's finished. */ flush_scheduled_work(); + vio_unregister_driver(&veth_driver); + for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) veth_destroy_connection(i);