niu: panic on reset

The reset_task function in the niu driver does not reset the hardware fully. The buffer addresses and pointers are not being reset/updated in the hardware and that leads to panic on reset.

---
drivers/net/niu.c | 16 ++++++++++++++++

1 files changed, 16 insertions(+), 0 deletions(-)

index e4765b7..422d7e5 100644
@@ -6006,11 +6006,27 @@ static void niu_reset_task(struct work_struct *work)
 
       niu_stop_hw(np);
 
+       spin_unlock_irqrestore(&np->lock, flags);
+
+       niu_free_channels(np);
+
+       err = niu_alloc_channels(np);
+       if (err) {
+               dev_err(np->device, PFX "%s:Failed to alloc niu channels\n",
+                       np->dev->name);
+               return;
+       }
+
+       spin_lock_irqsave(&np->lock, flags);
+
       err = niu_init_hw(np);
       if (!err) {
               np->timer.expires = jiffies + HZ;
               add_timer(&np->timer);
               niu_netif_start(np);
+       } else {
+               dev_err(np->device, PFX "%s:Failed to init hw\n",
+                       np->dev->name);
       }
 
       spin_unlock_irqrestore(&np->lock, flags);

--
1.5.2

From: [email protected]
Date: Thu, 04 Sep 2008 16:55:16 -0700

[ Please learn how to hit the return key every 80 characters or so,
long lines are very difficult to read on text-only email clients like the one I use. Expecting auto-formatting and other beautifications in email readers amongst kernel developers is a very bad idea. ]

This is way overkill just to fix this bug and it adds a new failure mode for the driver which is a seriously negative aspect to this fix.

Instead, simply write a function which reloads the buffers addresses properly instead of calling the functions to free and reallocate all of these dynamic datastructures just to get those side effects.

Dave,

Thanks for the feedback.
Instead of creating a new function, Santwona was borrowing an existing logic from niu_change_mtu function. Rather than introducing a new function and possibly a new failure mode, we were trying to re-use existing logic, reducing the probability of a new bug.

Regards,
Matheos

From: Matheos Worku <[email protected]>
Date: Wed, 10 Sep 2008 10:01:42 -0700

But in essence the patch does add a new failure mode. It now can fail if a memory allocation cannot succeed at the current point in time.

That was not true previously, and if you just fix up the buffer pointers or whatever state to be reset, you won't have this possible failure mode either.

We should fix this correctly.