From f27eff1ffd65236b8e421188f76ad1b0b94e06eb Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:47:27 +1000 Subject: [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 855f8b2..7d0ef29 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) -- cgit v1.1 From eb235aef724568ae15af831968000cf9a3974b26 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:53:18 +1000 Subject: [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 7d0ef29..1e869df 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 { -- cgit v1.1 From 41664c03f6c96a1f8a91714309b36f1b5ca85610 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 17:55:08 +1000 Subject: [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 1e869df..1edecb1 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; -- cgit v1.1 From b2e0852e1eee7c445b1789bef41204b64f981102 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 12 May 2005 18:09:45 +1000 Subject: [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 1edecb1..13ed8dc 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); -- cgit v1.1