gianfar.c null pointer deref in gfar_start_xmit().

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

Thanks,
Robin

On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

I was able to reproduce this with the net-next-2.6 kernel as well.

Robin

Le mardi 09 août 2011 à 01:54 -0500, Robin Holt a écrit :

On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:

quoted

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

I was able to reproduce this with the net-next-2.6 kernel as well.

This driver incorrectly assumes a non dense txqueue array is possible
for a netdev, but its not true.

In the meantime, you could force it to use one tx_queue only.

tx_queues = (u32 *)of_get_property(np, "fsl,num_tx_queues", NULL);
num_tx_qs = tx_queues ? *tx_queues : 1;

On Tue, Aug 09, 2011 at 09:10:13AM +0200, Eric Dumazet wrote:

Le mardi 09 août 2011 à 01:54 -0500, Robin Holt a écrit :

quoted

On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:

quoted

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

I was able to reproduce this with the net-next-2.6 kernel as well.

This driver incorrectly assumes a non dense txqueue array is possible
for a netdev, but its not true.

In the meantime, you could force it to use one tx_queue only.

tx_queues = (u32 *)of_get_property(np, "fsl,num_tx_queues", NULL);
num_tx_qs = tx_queues ? *tx_queues : 1;

I fixed up the .dts file and now it works.  I have not tested the RGMII
interface yet so I do not know if that supports multiple queues.  I assume
I could find it in the documentation.  What might I be looking for?

Thanks,
Robin

Le mardi 09 août 2011 à 08:06 -0500, Robin Holt a écrit :

On Tue, Aug 09, 2011 at 09:10:13AM +0200, Eric Dumazet wrote:

quoted

Le mardi 09 août 2011 à 01:54 -0500, Robin Holt a écrit :

quoted

On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:

quoted

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

I was able to reproduce this with the net-next-2.6 kernel as well.

This driver incorrectly assumes a non dense txqueue array is possible
for a netdev, but its not true.

In the meantime, you could force it to use one tx_queue only.

tx_queues = (u32 *)of_get_property(np, "fsl,num_tx_queues", NULL);
num_tx_qs = tx_queues ? *tx_queues : 1;

I fixed up the .dts file and now it works.  I have not tested the RGMII
interface yet so I do not know if that supports multiple queues.  I assume
I could find it in the documentation.  What might I be looking for?

What exact change did you in dts ?

Assuming your p1010rdb is single core powerpc, I fail to see why 8
queues should be enabled on NIC. It only increases number of interrupts
if multiple flows are in use, and memory footprint.

On Wed, Aug 10, 2011 at 08:01:01AM +0200, Eric Dumazet wrote:

Le mardi 09 août 2011 à 08:06 -0500, Robin Holt a écrit :

quoted

On Tue, Aug 09, 2011 at 09:10:13AM +0200, Eric Dumazet wrote:

quoted

Le mardi 09 août 2011 à 01:54 -0500, Robin Holt a écrit :

quoted

On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:

quoted

While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
(None which affect gianfar.c), I get a NULL pointer deref at:

static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
...
	regs = tx_queue->grp->regs;

I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
I have not looked at this any more than that.

Any suggestions would be welcome.   To reproduce, all I need to do is
a few sequences of pings.

I was able to reproduce this with the net-next-2.6 kernel as well.

This driver incorrectly assumes a non dense txqueue array is possible
for a netdev, but its not true.

In the meantime, you could force it to use one tx_queue only.

tx_queues = (u32 *)of_get_property(np, "fsl,num_tx_queues", NULL);
num_tx_qs = tx_queues ? *tx_queues : 1;

I fixed up the .dts file and now it works.  I have not tested the RGMII
interface yet so I do not know if that supports multiple queues.  I assume
I could find it in the documentation.  What might I be looking for?

What exact change did you in dts ?

I changed arch/powerpc/boot/dts/p1010si.dtsi and modified all lines that
had fsl,num_rx_queues and fsl,num_tx_queues then recompiled and rebuilt
my multi-image boot file.

It looks like my boot loader might be missing something.  The p1010
reference manual has a section 15.7.2 Multigroup Mode Initialization.
I assume that sections initialize before using multigroup mode steps are
not getting done in the boot loader.  Does that possibly sound like the
right section of the manual?

Assuming your p1010rdb is single core powerpc, I fail to see why 8
queues should be enabled on NIC. It only increases number of interrupts
if multiple flows are in use, and memory footprint.

Thanks,
Robin