Re: [Bugme-new] [Bug 9382] New: etting MTU > 1500 on card "cold" sigsegs the "ip" program and produces an OOPS.

Re: [Bugme-new] [Bug 9382] New: etting MTU > 1500 on card "cold" sigsegs the "ip" program and produces an OOPS.

[Andrew Morton]

On Wed, 14 Nov 2007 19:11:38 -0800 (PST) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9382
> 
>            Summary: etting MTU > 1500 on card "cold" sigsegs the "ip"
>                     program and produces an OOPS.
>            Product: Drivers
>            Version: 2.5
>      KernelVersion: VIA Velocity > 1500 MTU cold configure -=> OOPS
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: jgarzik@pobox.com
>         ReportedBy: jnelson-kernel-bugzilla@jamponi.net
> 
> 
> Most recent kernel where this bug did not occur: Still occurs on latest
> openSUSE 10.3 kernel. A brief scan of the git log for this driver does not
> suggest a fix.
> 
> Distribution: openSUSE 10.3
> Hardware Environment: AMD Athlon XP 2200
> Software Environment: kernel 2.6.22.12
> Problem Description: setting MTU > 1500 on card "cold" sigsegs the "ip" program
> and produces an OOPS.
> 
> Steps to reproduce: 
> 
> 1. have a VIA Velocity gig-e NIC, *cold* (unconfigured but module loaded).
> 2. Issue:
> 
> ip link set eth0 up 7200 192.168.1.1
> 
> This has been an issue since at least 2.6.18 but I wasn't able to reproduce it
> on demand for a whole variety of reasons. Now I am able to reproduce it, 100%
> of the time, on-demand (although this requires a reboot.)
> 
> 
> BUG: unable to handle kernel NULL pointer dereference at virtual address
> 00000003
>  printing eip:
> f96a7772
> *pde = 00000000
> Oops: 0000 [#1]
> SMP
> last sysfs file: /devices/pci0000:00/0000:00:00.0/class
> Modules linked in: xt_tcpudp xt_pkttype ipt_LOG xt_limit drbd snd_pcm_oss
> snd_mixer_oss snd_seq snd_seq_device ipt_REJECT xt_state iptable_mangle
> iptable_nat nf_nat iptable_filter nf_conntrack_ipv4 nf_conntrack nfnetlink
> ip_tables ip6_tables x_tables tcp_bic apparmor dm_crypt loop dm_mirror dm_log
> dm_mod snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm button parport_pc rtc_cmos
> via_velocity sr_mod cdrom snd_timer rtc_core parport snd crc_ccitt rtc_lib
> soundcore snd_page_alloc shpchp sis_agp pci_hotplug agpgart i2c_sis630
> i2c_sis96x i2c_core sg usbhid hid ff_memless ehci_hcd sd_mod ohci_hcd usbcore
> piix sis5513 ide_core edd ext3 mbcache jbd fan pata_sis libata scsi_mod thermal
> processor
> CPU:    0
> EIP:    0060:[<f96a7772>]    Tainted: G      N VLI
> EFLAGS: 00010046   (2.6.22.12-0.1-default #1)
> EIP is at velocity_rx_refill+0x22/0x1d1 [via_velocity]
> eax: c197a500   ebx: c197a500   ecx: 00000000   edx: df8a9c00
> esi: c197a500   edi: 00000000   ebp: 00000000   esp: f774fe84
> ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
> Process ip (pid: 4133, ti=f774e000 task=f7a31030 task.ti=f774e000)
> Stack: f774fef0 00000086 3f257025 c170a2a0 00000000 fffffff4 00000200 c197a500
>        df8a9e00 fffffff4 f96a798f c01d0360 f96a61a7 c197a500 c197a000 00001c20
>        00000000 f96a8955 00008922 00008922 00000216 c197a000 00000001 00008922
> Call Trace:
>  [<f96a798f>] velocity_init_rd_ring+0x6e/0xa0 [via_velocity]
>  [<c01d0360>] __delay+0x6/0x7
>  [<f96a61a7>] safe_disable_mii_autopoll+0x14/0x26 [via_velocity]
>  [<f96a8955>] velocity_change_mtu+0xbc/0x104 [via_velocity]
>  [<c026e62a>] dev_set_mtu+0x2d/0x5d
>  [<c0270788>] dev_ioctl+0x3b1/0x43e
>  [<c012daa8>] ptrace_notify+0x6a/0x94
>  [<c026554b>] sock_ioctl+0x19f/0x1be
>  [<c02653ac>] sock_ioctl+0x0/0x1be
>  [<c017a131>] do_ioctl+0x21/0xa0
>  [<c014dcc0>] audit_syscall_entry+0x105/0x13e
>  [<c017a3e7>] vfs_ioctl+0x237/0x249
>  [<c017a445>] sys_ioctl+0x4c/0x67
>  [<c0104ea2>] syscall_call+0x7/0xb
>  =======================
> Code: ff 31 c0 5a 5b 5e 5f 5d c3 55 57 56 53 89 c3 83 ec 18 8b a8 f8 00 00 00
> c7 44 24 10 00 00 00 00 89 ef c1 e7 04 03 bb 00 01 00 00 <80> 7f 03 00 0f 88 1d
> 01 00 00 8d 34 ed 00 00 00 00 03 b3 04 01
> EIP: [<f96a7772>] velocity_rx_refill+0x22/0x1d1 [via_velocity] SS:ESP
> 0068:f774fe84
> 

[PATCH] via-velocity: don't oops on MTU change.

[Stephen Hemminger]

Simple mtu change when device is down.
Fix http://bugzilla.kernel.org/show_bug.cgi?id=9382.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>


--- a/drivers/net/via-velocity.c	2007-10-22 09:38:11.000000000 -0700
+++ b/drivers/net/via-velocity.c	2007-11-14 19:34:30.000000000 -0800
@@ -1963,6 +1963,11 @@ static int velocity_change_mtu(struct ne
 		return -EINVAL;
 	}
 
+	if (!netif_running(dev)) {
+		dev->mtu = new_mtu;
+		return 0;
+	}
+
 	if (new_mtu != oldmtu) {
 		spin_lock_irqsave(&vptr->lock, flags);
 

Re: [PATCH] via-velocity: don't oops on MTU change.

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed, 14 Nov 2007 19:38:44 -0800

> Simple mtu change when device is down.
> Fix http://bugzilla.kernel.org/show_bug.cgi?id=9382.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Applied.

Thanks for looking at this bug so quickly Stephen.

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jarek Poplawski]

On 15-11-2007 04:38, Stephen Hemminger wrote:
> Simple mtu change when device is down.
> Fix http://bugzilla.kernel.org/show_bug.cgi?id=9382.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> 
> --- a/drivers/net/via-velocity.c	2007-10-22 09:38:11.000000000 -0700
> +++ b/drivers/net/via-velocity.c	2007-11-14 19:34:30.000000000 -0800
> @@ -1963,6 +1963,11 @@ static int velocity_change_mtu(struct ne
>  		return -EINVAL;
>  	}
>  
> +	if (!netif_running(dev)) {
> +		dev->mtu = new_mtu;
> +		return 0;
> +	}
> +
>  	if (new_mtu != oldmtu) {
>  		spin_lock_irqsave(&vptr->lock, flags);

Shouldn't this latter 'if' be removed now, btw?

Regards,
Jarek P.

Re: [PATCH] via-velocity: don't oops on MTU change.

[Stephen Hemminger]

On Thu, 15 Nov 2007 09:26:00 +0100
Jarek Poplawski <jarkao2@o2.pl> wrote:

> On 15-11-2007 04:38, Stephen Hemminger wrote:
> > Simple mtu change when device is down.
> > Fix http://bugzilla.kernel.org/show_bug.cgi?id=9382.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> > 
> > 
> > --- a/drivers/net/via-velocity.c	2007-10-22 09:38:11.000000000 -0700
> > +++ b/drivers/net/via-velocity.c	2007-11-14 19:34:30.000000000 -0800
> > @@ -1963,6 +1963,11 @@ static int velocity_change_mtu(struct ne
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (!netif_running(dev)) {
> > +		dev->mtu = new_mtu;
> > +		return 0;
> > +	}
> > +
> >  	if (new_mtu != oldmtu) {
> >  		spin_lock_irqsave(&vptr->lock, flags);
> 
> Shouldn't this latter 'if' be removed now, btw?

No, it makes sense that if mtu is same, no action need be taken.

Actually, it would make sense to push the same check up into
the netdevice core management.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jarek Poplawski]

Stephen Hemminger wrote, On 11/15/2007 07:20 PM:

> On Thu, 15 Nov 2007 09:26:00 +0100
> Jarek Poplawski <jarkao2@o2.pl> wrote:
> 
>> On 15-11-2007 04:38, Stephen Hemminger wrote:
>>> Simple mtu change when device is down.
>>> Fix http://bugzilla.kernel.org/show_bug.cgi?id=9382.
>>>
>>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>>>
>>>
>>> --- a/drivers/net/via-velocity.c	2007-10-22 09:38:11.000000000 -0700
>>> +++ b/drivers/net/via-velocity.c	2007-11-14 19:34:30.000000000 -0800
>>> @@ -1963,6 +1963,11 @@ static int velocity_change_mtu(struct ne
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> +	if (!netif_running(dev)) {
>>> +		dev->mtu = new_mtu;
>>> +		return 0;
>>> +	}
>>> +
>>>  	if (new_mtu != oldmtu) {
>>>  		spin_lock_irqsave(&vptr->lock, flags);
>> Shouldn't this latter 'if' be removed now, btw?
> 
> No, it makes sense that if mtu is same, no action need be taken.
> 
> Actually, it would make sense to push the same check up into
> the netdevice core management.

Sure! I was only worried velocity_open() treats dev->mtu
a bit different than velocity_change_mtu(), so eg. after:

velocity_change_mtu() // dev is down
velocity_open()
velocity_change_mtu() // dev is up

with the same mtu, vptr->rx_buf_sz could be different than after:

velocity_open()
velocity_change_mtu() // dev is up

But, probably, I miss someting.

Thanks,
Jarek P.

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jon Nelson]

On 11/15/07, Jarek Poplawski <jarkao2@o2.pl> wrote:
..

> Sure! I was only worried velocity_open() treats dev->mtu
> a bit different than velocity_change_mtu(), so eg. after:
>
> velocity_change_mtu() // dev is down
> velocity_open()
> velocity_change_mtu() // dev is up
>
> with the same mtu, vptr->rx_buf_sz could be different than after:
>
> velocity_open()
> velocity_change_mtu() // dev is up
>
> But, probably, I miss someting.

There is a snag here: if I change the MTU after the device is UP
something ends up rather broken. A tcpdump shows nearly every outgoing
frame has a bad TCP checksum (and the card does not support H/W
checksumming or it is turned off as reported by ethtool).

-- 
Jon

Re: [PATCH] via-velocity: don't oops on MTU change.

[Stephen Hemminger]

On Thu, 15 Nov 2007 20:42:55 -0600
"Jon Nelson" <jnelson@jamponi.net> wrote:

> On 11/15/07, Jarek Poplawski <jarkao2@o2.pl> wrote:
> ..
> 
> > Sure! I was only worried velocity_open() treats dev->mtu
> > a bit different than velocity_change_mtu(), so eg. after:
> >
> > velocity_change_mtu() // dev is down
> > velocity_open()
> > velocity_change_mtu() // dev is up
> >
> > with the same mtu, vptr->rx_buf_sz could be different than after:
> >
> > velocity_open()
> > velocity_change_mtu() // dev is up
> >
> > But, probably, I miss someting.
> 
> There is a snag here: if I change the MTU after the device is UP
> something ends up rather broken. A tcpdump shows nearly every outgoing
> frame has a bad TCP checksum (and the card does not support H/W
> checksumming or it is turned off as reported by ethtool).
> 

That is a different (and pre-existing bug). 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] via-velocity: don't oops on MTU change.

[Stephen Hemminger]

On Thu, 15 Nov 2007 20:42:55 -0600
"Jon Nelson" <jnelson@jamponi.net> wrote:

> On 11/15/07, Jarek Poplawski <jarkao2@o2.pl> wrote:
> ..
> 
> > Sure! I was only worried velocity_open() treats dev->mtu
> > a bit different than velocity_change_mtu(), so eg. after:
> >
> > velocity_change_mtu() // dev is down
> > velocity_open()
> > velocity_change_mtu() // dev is up
> >
> > with the same mtu, vptr->rx_buf_sz could be different than after:
> >
> > velocity_open()
> > velocity_change_mtu() // dev is up
> >
> > But, probably, I miss someting.
> 
> There is a snag here: if I change the MTU after the device is UP
> something ends up rather broken. A tcpdump shows nearly every outgoing
> frame has a bad TCP checksum (and the card does not support H/W
> checksumming or it is turned off as reported by ethtool).
> 

Does this fix the problem.

Note: reading the code the driver has other problems (besides crappy
style). It does pci_map_single()/unmap in a way that doesn't account
correctly for the padding that was added.

--- a/drivers/net/via-velocity.c	2007-11-15 20:11:12.000000000 -0800
+++ b/drivers/net/via-velocity.c	2007-11-15 20:32:14.000000000 -0800
@@ -1242,6 +1242,9 @@ static int velocity_rx_refill(struct vel
 static int velocity_init_rd_ring(struct velocity_info *vptr)
 {
 	int ret;
+	int mtu = vptr->dev->mtu;
+
+	vptr->rx_buf_sz = (mtu <= ETH_DATA_LEN) ? PKT_BUF_SZ : mtu + 32;
 
 	vptr->rd_info = kcalloc(vptr->options.numrx,
 				sizeof(struct velocity_rd_info), GFP_KERNEL);
@@ -1898,8 +1901,6 @@ static int velocity_open(struct net_devi
 	struct velocity_info *vptr = netdev_priv(dev);
 	int ret;
 
-	vptr->rx_buf_sz = (dev->mtu <= 1504 ? PKT_BUF_SZ : dev->mtu + 32);
-
 	ret = velocity_init_rings(vptr);
 	if (ret < 0)
 		goto out;
@@ -1978,12 +1979,6 @@ static int velocity_change_mtu(struct ne
 		velocity_free_rd_ring(vptr);
 
 		dev->mtu = new_mtu;
-		if (new_mtu > 8192)
-			vptr->rx_buf_sz = 9 * 1024;
-		else if (new_mtu > 4096)
-			vptr->rx_buf_sz = 8192;
-		else
-			vptr->rx_buf_sz = 4 * 1024;
 
 		ret = velocity_init_rd_ring(vptr);
 		if (ret < 0)


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jon Nelson]

I get a segmentation fault.

Should this patch be in addition to the one from bug 9382?
http://bugzilla.kernel.org/attachment.cgi?id=13555&action=view

-- 
Jon

Re: [PATCH] via-velocity: don't oops on MTU change.

[Stephen Hemminger]

On Fri, 16 Nov 2007 10:21:25 -0600
"Jon Nelson" <jnelson@jamponi.net> wrote:

> I get a segmentation fault.
> 
> Should this patch be in addition to the one from bug 9382?
> http://bugzilla.kernel.org/attachment.cgi?id=13555&action=view
> 
Yes. The new patch adds just recomputes the rx buf size in
one place, so it can be set when device is down.
Also, the computation will always yield same result now, the old
code computed different values depending on whether it was initial
or changed MTU.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] via-velocity: don't oops on MTU change.

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 16 Nov 2007 08:47:06 -0800

> On Fri, 16 Nov 2007 10:21:25 -0600
> "Jon Nelson" <jnelson@jamponi.net> wrote:
> 
> > I get a segmentation fault.
> > 
> > Should this patch be in addition to the one from bug 9382?
> > http://bugzilla.kernel.org/attachment.cgi?id=13555&action=view
> > 
> Yes. The new patch adds just recomputes the rx buf size in
> one place, so it can be set when device is down.
> Also, the computation will always yield same result now, the old
> code computed different values depending on whether it was initial
> or changed MTU.

But with your patch he is getting an OOPS (he described it as
a 'segmentation fault' which is why you perhaps missed it)
so you should try to figure out why so we can fix this bug.

:-)

Thanks.

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jon Nelson]

On 11/16/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> On Fri, 16 Nov 2007 10:21:25 -0600
> "Jon Nelson" <jnelson@jamponi.net> wrote:
>
> > I get a segmentation fault.
> >
> > Should this patch be in addition to the one from bug 9382?
> > http://bugzilla.kernel.org/attachment.cgi?id=13555&action=view
> >
> Yes. The new patch adds just recomputes the rx buf size in
> one place, so it can be set when device is down.
> Also, the computation will always yield same result now, the old
> code computed different values depending on whether it was initial
> or changed MTU.

OK. This is what I did.
Using git I grabbed a copy of Linus' tree and using the latest files
for via-velocity.[c,h], commit
99fee6d7e5748d96884667a4628118f7fc130ea0, I determined that if I
backed out change 44c10138fd4bbc4b6d6bff0873c24902f2a9da65 (PCI:
Change all drivers to use pci_device->revision) I could get it to
compile. This gets me a more recent driver.

Then I applied both of the patches you have provided me, and built and
tried that.
No sigseg, no oops on initial MTU, no sigseg or oops on subsequent MTU changes.

Performance appears to be worse after MTU changes, though. I can get
about 30MB/s with 7200 (the largest the board will support?) and about
5MB/s with 1500. If I *start out* with 1500 performance is 35-40MB/s.
However, the immediate issue appears to be solved (oopses).

If there is a better tree to pull from, I'm more than willing to keep
trying different drivers, at least for a while. I also have tg3 driver
issues to sort out.

-- 
Jon

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jarek Poplawski]

Jon Nelson wrote, On 11/17/2007 01:59 AM:
...

> OK. This is what I did.
> Using git I grabbed a copy of Linus' tree and using the latest files
> for via-velocity.[c,h], commit
> 99fee6d7e5748d96884667a4628118f7fc130ea0, I determined that if I
> backed out change 44c10138fd4bbc4b6d6bff0873c24902f2a9da65 (PCI:
> Change all drivers to use pci_device->revision) I could get it to
> compile. This gets me a more recent driver.
> 
> Then I applied both of the patches you have provided me, and built and
> tried that.
> No sigseg, no oops on initial MTU, no sigseg or oops on subsequent MTU changes.


Good news! But, if I got it right your method could be tricky. These
current via-velocity files could depend on other files being current
as well. So, it's safer to use the whole new kernel (eg. 2.6.24-rc3)
or to stay with your older one. But, if you want to check the effect
of these new two patches only without any additional 'features', your
older kernel should be a better choice.

Regards,
Jarek P.

Re: [PATCH] via-velocity: don't oops on MTU change.

[Jon Nelson]

Just a reminder: the following fix should (IMO) be considered for
2.6.24. The first patch does indeed fix the OOPS on MTU however
subsequent MTU changes still OOPS. The below fixes that. I've been
using both patches for over a week and they work just great. I noticed
that the first fix made 2.6.24 but this one hadn't -  I just want to
try to make sure 2.6.24 doesn't make things /worse/ for via-velocity
owners instead of better.

Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> Does this fix the problem.
>
> Note: reading the code the driver has other problems (besides crappy
> style). It does pci_map_single()/unmap in a way that doesn't account
> correctly for the padding that was added.
>
> --- a/drivers/net/via-velocity.c        2007-11-15 20:11:12.000000000 -0800
> +++ b/drivers/net/via-velocity.c        2007-11-15 20:32:14.000000000 -0800
> @@ -1242,6 +1242,9 @@ static int velocity_rx_refill(struct vel
>  static int velocity_init_rd_ring(struct velocity_info *vptr)
>  {
>         int ret;
> +       int mtu = vptr->dev->mtu;
> +
> +       vptr->rx_buf_sz = (mtu <= ETH_DATA_LEN) ? PKT_BUF_SZ : mtu + 32;
>
>         vptr->rd_info = kcalloc(vptr->options.numrx,
>                                 sizeof(struct velocity_rd_info), GFP_KERNEL);
> @@ -1898,8 +1901,6 @@ static int velocity_open(struct net_devi
>         struct velocity_info *vptr = netdev_priv(dev);
>         int ret;
>
> -       vptr->rx_buf_sz = (dev->mtu <= 1504 ? PKT_BUF_SZ : dev->mtu + 32);
> -
>         ret = velocity_init_rings(vptr);
>         if (ret < 0)
>                 goto out;
> @@ -1978,12 +1979,6 @@ static int velocity_change_mtu(struct ne
>                 velocity_free_rd_ring(vptr);
>
>                 dev->mtu = new_mtu;
> -               if (new_mtu > 8192)
> -                       vptr->rx_buf_sz = 9 * 1024;
> -               else if (new_mtu > 4096)
> -                       vptr->rx_buf_sz = 8192;
> -               else
> -                       vptr->rx_buf_sz = 4 * 1024;
>
>                 ret = velocity_init_rd_ring(vptr);
>                 if (ret < 0)

-- 
Jon

[PATCH] via-velocity: don't oops on MTU change (resend)

[Stephen Hemminger]

The VIA veloicty driver needs the following to allow changing MTU when down.
The buffer size needs to be computed when device is brought up, not when
device is initialized.  This also fixes a bug where the buffer size was
computed differently on change_mtu versus initial setting.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

---
This is a properly formatted version of previously submitted patch.
Please apply for 2.6.24

--- a/drivers/net/via-velocity.c	2007-11-15 20:11:12.000000000 -0800
+++ b/drivers/net/via-velocity.c	2007-11-15 20:32:14.000000000 -0800
@@ -1242,6 +1242,9 @@ static int velocity_rx_refill(struct vel
 static int velocity_init_rd_ring(struct velocity_info *vptr)
 {
 	int ret;
+	int mtu = vptr->dev->mtu;
+
+	vptr->rx_buf_sz = (mtu <= ETH_DATA_LEN) ? PKT_BUF_SZ : mtu + 32;
 
 	vptr->rd_info = kcalloc(vptr->options.numrx,
 				sizeof(struct velocity_rd_info), GFP_KERNEL);
@@ -1898,8 +1901,6 @@ static int velocity_open(struct net_devi
 	struct velocity_info *vptr = netdev_priv(dev);
 	int ret;
 
-	vptr->rx_buf_sz = (dev->mtu <= 1504 ? PKT_BUF_SZ : dev->mtu + 32);
-
 	ret = velocity_init_rings(vptr);
 	if (ret < 0)
 		goto out;
@@ -1978,12 +1979,6 @@ static int velocity_change_mtu(struct ne
 		velocity_free_rd_ring(vptr);
 
 		dev->mtu = new_mtu;
-		if (new_mtu > 8192)
-			vptr->rx_buf_sz = 9 * 1024;
-		else if (new_mtu > 4096)
-			vptr->rx_buf_sz = 8192;
-		else
-			vptr->rx_buf_sz = 4 * 1024;
 
 		ret = velocity_init_rd_ring(vptr);
 		if (ret < 0)

Re: [PATCH] via-velocity: don't oops on MTU change (resend)

[Jeff Garzik]

Stephen Hemminger wrote:
> The VIA veloicty driver needs the following to allow changing MTU when down.
> The buffer size needs to be computed when device is brought up, not when
> device is initialized.  This also fixes a bug where the buffer size was
> computed differently on change_mtu versus initial setting.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> ---
> This is a properly formatted version of previously submitted patch.
> Please apply for 2.6.24

applied