linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] e1000 C style badness
       [not found] <5wgyi-18w-7@gated-at.bofh.it>
@ 2006-01-18  8:56 ` Patrizio Bassi
  2006-01-18  9:00   ` Jens Axboe
  2006-01-18  9:02   ` Nick Piggin
  0 siblings, 2 replies; 13+ messages in thread
From: Patrizio Bassi @ 2006-01-18  8:56 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Kernel, 

Jens Axboe ha scritto:
> Hi,
> 
> Recent e1000 updates introduced variable declarations after code. Fix
> those up again.
> 
> Signed-off-by: Jens Axboe <axboe@suse.de>
> 
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index d0a5d16..ca68a04 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
>  		e1000_pci_set_mwi(&adapter->hw);
>  
>  	if(netif_running(netdev)) {
> +		struct e1000_rx_ring *ring;
> +
>  		e1000_configure_rx(adapter);
>  		/* No need to loop, because 82542 supports only 1 queue */
> -		struct e1000_rx_ring *ring = &adapter->rx_ring[0];
> +		ring = &adapter->rx_ring[0];
>  		adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
>  	}
>  }
> @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter 
>  	rx_desc = E1000_RX_DESC(*rx_ring, i);
>  
>  	while(rx_desc->status & E1000_RXD_STAT_DD) {
> -		buffer_info = &rx_ring->buffer_info[i];
>  		u8 status;
> +		buffer_info = &rx_ring->buffer_info[i];
>  #ifdef CONFIG_E1000_NAPI
>  		if(*work_done >= work_to_do)
>  			break;
> 

Shouldn't variables declaration be on top of function and not on top of
a block (like if, while, for...)?

--
Patrizio Bassi
www.patriziobassi.it

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18  8:56 ` [PATCH] e1000 C style badness Patrizio Bassi
@ 2006-01-18  9:00   ` Jens Axboe
  2006-01-18  9:02   ` Nick Piggin
  1 sibling, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2006-01-18  9:00 UTC (permalink / raw)
  To: Patrizio Bassi; +Cc: Kernel, 

On Wed, Jan 18 2006, Patrizio Bassi wrote:
> Jens Axboe ha scritto:
> > Hi,
> > 
> > Recent e1000 updates introduced variable declarations after code. Fix
> > those up again.
> > 
> > Signed-off-by: Jens Axboe <axboe@suse.de>
> > 
> > diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> > index d0a5d16..ca68a04 100644
> > --- a/drivers/net/e1000/e1000_main.c
> > +++ b/drivers/net/e1000/e1000_main.c
> > @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
> >  		e1000_pci_set_mwi(&adapter->hw);
> >  
> >  	if(netif_running(netdev)) {
> > +		struct e1000_rx_ring *ring;
> > +
> >  		e1000_configure_rx(adapter);
> >  		/* No need to loop, because 82542 supports only 1 queue */
> > -		struct e1000_rx_ring *ring = &adapter->rx_ring[0];
> > +		ring = &adapter->rx_ring[0];
> >  		adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
> >  	}
> >  }
> > @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter 
> >  	rx_desc = E1000_RX_DESC(*rx_ring, i);
> >  
> >  	while(rx_desc->status & E1000_RXD_STAT_DD) {
> > -		buffer_info = &rx_ring->buffer_info[i];
> >  		u8 status;
> > +		buffer_info = &rx_ring->buffer_info[i];
> >  #ifdef CONFIG_E1000_NAPI
> >  		if(*work_done >= work_to_do)
> >  			break;
> > 
> 
> Shouldn't variables declaration be on top of function and not on top of
> a block (like if, while, for...)?

No, that's not necessary. But they should be before actual code inside
that block.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18  8:56 ` [PATCH] e1000 C style badness Patrizio Bassi
  2006-01-18  9:00   ` Jens Axboe
@ 2006-01-18  9:02   ` Nick Piggin
  2006-01-18 11:30     ` Patrizio Bassi
  1 sibling, 1 reply; 13+ messages in thread
From: Nick Piggin @ 2006-01-18  9:02 UTC (permalink / raw)
  To: patrizio.bassi; +Cc: Jens Axboe, linux-kernel

Patrizio Bassi wrote:
> Jens Axboe ha scritto:
> 
>>Hi,
>>
>>Recent e1000 updates introduced variable declarations after code. Fix
>>those up again.
>>
>>Signed-off-by: Jens Axboe <axboe@suse.de>
>>
>>diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
>>index d0a5d16..ca68a04 100644
>>--- a/drivers/net/e1000/e1000_main.c
>>+++ b/drivers/net/e1000/e1000_main.c
>>@@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
>> 		e1000_pci_set_mwi(&adapter->hw);
>> 
>> 	if(netif_running(netdev)) {
>>+		struct e1000_rx_ring *ring;
>>+
>> 		e1000_configure_rx(adapter);
>> 		/* No need to loop, because 82542 supports only 1 queue */
>>-		struct e1000_rx_ring *ring = &adapter->rx_ring[0];
>>+		ring = &adapter->rx_ring[0];
>> 		adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
>> 	}
>> }
>>@@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter 
>> 	rx_desc = E1000_RX_DESC(*rx_ring, i);
>> 
>> 	while(rx_desc->status & E1000_RXD_STAT_DD) {
>>-		buffer_info = &rx_ring->buffer_info[i];
>> 		u8 status;
>>+		buffer_info = &rx_ring->buffer_info[i];
>> #ifdef CONFIG_E1000_NAPI
>> 		if(*work_done >= work_to_do)
>> 			break;
>>
> 
> 
> Shouldn't variables declaration be on top of function and not on top of
> a block (like if, while, for...)?
> 

Any block is OK, and they all have the same nice symmetry - variables
come into scope at the top and go out of scope at the bottom.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18  9:02   ` Nick Piggin
@ 2006-01-18 11:30     ` Patrizio Bassi
  0 siblings, 0 replies; 13+ messages in thread
From: Patrizio Bassi @ 2006-01-18 11:30 UTC (permalink / raw)
  To: Nick Piggin; +Cc: Jens Axboe, linux-kernel

Nick Piggin ha scritto:
> Patrizio Bassi wrote:
>> Jens Axboe ha scritto:
>>
>>> Hi,
>>>
>>> Recent e1000 updates introduced variable declarations after code. Fix
>>> those up again.
>>>
>>> Signed-off-by: Jens Axboe <axboe@suse.de>
>>>
>>> diff --git a/drivers/net/e1000/e1000_main.c
>>> b/drivers/net/e1000/e1000_main.c
>>> index d0a5d16..ca68a04 100644
>>> --- a/drivers/net/e1000/e1000_main.c
>>> +++ b/drivers/net/e1000/e1000_main.c
>>> @@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
>>>         e1000_pci_set_mwi(&adapter->hw);
>>>
>>>     if(netif_running(netdev)) {
>>> +        struct e1000_rx_ring *ring;
>>> +
>>>         e1000_configure_rx(adapter);
>>>         /* No need to loop, because 82542 supports only 1 queue */
>>> -        struct e1000_rx_ring *ring = &adapter->rx_ring[0];
>>> +        ring = &adapter->rx_ring[0];
>>>         adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
>>>     }
>>> }
>>> @@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter
>>>     rx_desc = E1000_RX_DESC(*rx_ring, i);
>>>
>>>     while(rx_desc->status & E1000_RXD_STAT_DD) {
>>> -        buffer_info = &rx_ring->buffer_info[i];
>>>         u8 status;
>>> +        buffer_info = &rx_ring->buffer_info[i];
>>> #ifdef CONFIG_E1000_NAPI
>>>         if(*work_done >= work_to_do)
>>>             break;
>>>
>>
>>
>> Shouldn't variables declaration be on top of function and not on top of
>> a block (like if, while, for...)?
>>
>
> Any block is OK, and they all have the same nice symmetry - variables
> come into scope at the top and go out of scope at the bottom.
>
ok, i'm still linked to the 70' C style (not confortable) :)
old compilers failed with such syntax.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-21  6:52             ` Jesse Brandeburg
@ 2006-01-21 11:42               ` Jens Axboe
  0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2006-01-21 11:42 UTC (permalink / raw)
  To: Jesse Brandeburg
  Cc: Jeff Garzik, Jon Smirl, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On Fri, Jan 20 2006, Jesse Brandeburg wrote:
> On 1/19/06, Jens Axboe <axboe@suse.de> wrote:
> > On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> > > just FYI, I have a patch for the e1000 breakage which will be out as
> > > soon as I can generate it.
> >
> > Newest -git works for me. Well sort of, I get a lot of these:
> >
> > e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> >   Tx Queue             <0>
> >   TDH                  <72>
> >   TDT                  <e5>
> >   next_to_use          <e5>
> >   next_to_clean        <6f>
> > buffer_info[next_to_clean]
> >   time_stamp           <10000160a>
> >   next_to_watch        <72>
> >   jiffies              <100001e09>
> >   next_to_watch.status <0>
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> >   Tx Queue             <0>
> >   TDH                  <72>
> >   TDT                  <e5>
> >   next_to_use          <e5>
> >   next_to_clean        <6f>
> > buffer_info[next_to_clean]
> >   time_stamp           <10000160a>
> >   next_to_watch        <72>
> >   jiffies              <1000025da>
> >   next_to_watch.status <0>
> > e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
> >   Tx Queue             <0>
> >   TDH                  <72>
> >   TDT                  <e5>
> >   next_to_use          <e5>
> >   next_to_clean        <6f>
> > buffer_info[next_to_clean]
> >   time_stamp           <10000160a>
> >   next_to_watch        <72>
> >   jiffies              <10000357b>
> >   next_to_watch.status <0>
> > NETDEV WATCHDOG: eth0: transmit timed out
> > e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
> 
> where it didn't happen with the previous driver?  I guess thats a good
> thing, kinda as we made the problem more frequent, hopefully we can
> help fix it?

Sorry I thought I had mentioned, it happens with the previous driver as
well. And it's pretty annoying, I get hickups due to these network
glitches.

> you don't happen to have TSO enabled do you?

Nope.

> please reply over at netdev at vger.kernel.org with the standard set
> of information, lspci, dmesg, etc etc.

Sure.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-19  8:02           ` Jens Axboe
@ 2006-01-21  6:52             ` Jesse Brandeburg
  2006-01-21 11:42               ` Jens Axboe
  0 siblings, 1 reply; 13+ messages in thread
From: Jesse Brandeburg @ 2006-01-21  6:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jeff Garzik, Jon Smirl, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On 1/19/06, Jens Axboe <axboe@suse.de> wrote:
> On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> > just FYI, I have a patch for the e1000 breakage which will be out as
> > soon as I can generate it.
>
> Newest -git works for me. Well sort of, I get a lot of these:
>
> e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
>   Tx Queue             <0>
>   TDH                  <72>
>   TDT                  <e5>
>   next_to_use          <e5>
>   next_to_clean        <6f>
> buffer_info[next_to_clean]
>   time_stamp           <10000160a>
>   next_to_watch        <72>
>   jiffies              <100001e09>
>   next_to_watch.status <0>
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
>   Tx Queue             <0>
>   TDH                  <72>
>   TDT                  <e5>
>   next_to_use          <e5>
>   next_to_clean        <6f>
> buffer_info[next_to_clean]
>   time_stamp           <10000160a>
>   next_to_watch        <72>
>   jiffies              <1000025da>
>   next_to_watch.status <0>
> e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
>   Tx Queue             <0>
>   TDH                  <72>
>   TDT                  <e5>
>   next_to_use          <e5>
>   next_to_clean        <6f>
> buffer_info[next_to_clean]
>   time_stamp           <10000160a>
>   next_to_watch        <72>
>   jiffies              <10000357b>
>   next_to_watch.status <0>
> NETDEV WATCHDOG: eth0: transmit timed out
> e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex

where it didn't happen with the previous driver?  I guess thats a good
thing, kinda as we made the problem more frequent, hopefully we can
help fix it?

you don't happen to have TSO enabled do you?

please reply over at netdev at vger.kernel.org with the standard set
of information, lspci, dmesg, etc etc.

Thanks,
  Jesse

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18 19:10         ` Jesse Brandeburg
@ 2006-01-19  8:02           ` Jens Axboe
  2006-01-21  6:52             ` Jesse Brandeburg
  0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2006-01-19  8:02 UTC (permalink / raw)
  To: Jesse Brandeburg
  Cc: Jeff Garzik, Jon Smirl, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On Wed, Jan 18 2006, Jesse Brandeburg wrote:
> On 1/18/06, Jens Axboe <axboe@suse.de> wrote:
> > On Wed, Jan 18 2006, Jeff Garzik wrote:
> > > We'll get it fixed ASAP, even if it means reverting all those patches.
> >
> > Thanks, it is rather critical. At least it didn't get included in -rc1,
> > that would have been a disaster :-)
> 
> just FYI, I have a patch for the e1000 breakage which will be out as
> soon as I can generate it.

Newest -git works for me. Well sort of, I get a lot of these:

e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
  Tx Queue             <0>
  TDH                  <72>
  TDT                  <e5>
  next_to_use          <e5>
  next_to_clean        <6f>
buffer_info[next_to_clean]
  time_stamp           <10000160a>
  next_to_watch        <72>
  jiffies              <100001e09>
  next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
  Tx Queue             <0>
  TDH                  <72>
  TDT                  <e5>
  next_to_use          <e5>
  next_to_clean        <6f>
buffer_info[next_to_clean]
  time_stamp           <10000160a>
  next_to_watch        <72>
  jiffies              <1000025da>
  next_to_watch.status <0>
e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang
  Tx Queue             <0>
  TDH                  <72>
  TDT                  <e5>
  next_to_use          <e5>
  next_to_clean        <6f>
buffer_info[next_to_clean]
  time_stamp           <10000160a>
  next_to_watch        <72>
  jiffies              <10000357b>
  next_to_watch.status <0>
NETDEV WATCHDOG: eth0: transmit timed out
e1000: eth0: e1000_watchdog_task: NIC Link is Up 1000 Mbps Full Duplex


-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18 18:20       ` Jens Axboe
@ 2006-01-18 19:10         ` Jesse Brandeburg
  2006-01-19  8:02           ` Jens Axboe
  0 siblings, 1 reply; 13+ messages in thread
From: Jesse Brandeburg @ 2006-01-18 19:10 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jeff Garzik, Jon Smirl, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On 1/18/06, Jens Axboe <axboe@suse.de> wrote:
> On Wed, Jan 18 2006, Jeff Garzik wrote:
> > We'll get it fixed ASAP, even if it means reverting all those patches.
>
> Thanks, it is rather critical. At least it didn't get included in -rc1,
> that would have been a disaster :-)

just FYI, I have a patch for the e1000 breakage which will be out as
soon as I can generate it.

Jesse

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18 18:13     ` Jeff Garzik
@ 2006-01-18 18:20       ` Jens Axboe
  2006-01-18 19:10         ` Jesse Brandeburg
  0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2006-01-18 18:20 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Jon Smirl, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On Wed, Jan 18 2006, Jeff Garzik wrote:
> Jon Smirl wrote:
> >e1000 is failing the same way for me too. Chip is on the motherboard:
> >Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)
> [...]
> >I am using an old switch so I normally get a half duplex connection:
> >Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
> >Link is Up 100 Mbps Half Duplex
> >I don't normally get the NETDEV up and changed notifications.
> 
> e1000 is failing for everybody :(  Looks like somewhere in this 40-patch 
> behemoth, an "e1000 doesn't work anymore" change slipped in.
> 
> We'll get it fixed ASAP, even if it means reverting all those patches.

Thanks, it is rather critical. At least it didn't get included in -rc1,
that would have been a disaster :-)

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18 17:53   ` Jon Smirl
@ 2006-01-18 18:13     ` Jeff Garzik
  2006-01-18 18:20       ` Jens Axboe
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Garzik @ 2006-01-18 18:13 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Jens Axboe, jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

Jon Smirl wrote:
> e1000 is failing the same way for me too. Chip is on the motherboard:
> Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)
[...]
> I am using an old switch so I normally get a half duplex connection:
> Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
> Link is Up 100 Mbps Half Duplex
> I don't normally get the NETDEV up and changed notifications.

e1000 is failing for everybody :(  Looks like somewhere in this 40-patch 
behemoth, an "e1000 doesn't work anymore" change slipped in.

We'll get it fixed ASAP, even if it means reverting all those patches.

	Jeff



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18  8:37 ` Jens Axboe
@ 2006-01-18 17:53   ` Jon Smirl
  2006-01-18 18:13     ` Jeff Garzik
  0 siblings, 1 reply; 13+ messages in thread
From: Jon Smirl @ 2006-01-18 17:53 UTC (permalink / raw)
  To: Jens Axboe; +Cc: jeffrey.t.kirsher, Linux Kernel, Linus Torvalds

On 1/18/06, Jens Axboe <axboe@suse.de> wrote:
> Hi,
>
> Seems e1000 has even bigger problems than just C style badness in the
> newest -git after the e1000 updates. diff of kernel boot messages from
> e1000:
>
> -Intel(R) PRO/1000 Network Driver - version 6.1.16-k2
> +Intel(R) PRO/1000 Network Driver - version 6.3.9-k2
>  Copyright (c) 1999-2005 Intel Corporation.
>  ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 16 (level, low) -> IRQ 169
>  PCI: Setting latency timer of device 0000:02:00.0 to 64
> +e1000: 0000:02:00.0: e1000_probe: (PCI Express:2.5Gb/s:Width x1)
> 00:0c:f1:ff:92:b8
>  e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
>
> It sends/receives 5 packages, but the dhcp request never succeeds. If I
> down the interface, it oopses (nvidia module is loaded, however it bombs
> in the same way without it). If I revert just the e1000 updates, my
> system work fine (typing this message from it).

e1000 is failing the same way for me too. Chip is on the motherboard:
Intel Corporation 82540EM Gigabit Ethernet Controller (rev 02)

Jan 18 10:08:26 jonsmirl kernel: ADDRCONF(NETDEV_UP): eth0: link is not ready
Jan 18 10:08:26 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
Link is Up 100 Mbps Half Duplex
Jan 18 10:08:26 jonsmirl kernel: ADDRCONF(NETDEV_CHANGE): eth0: link
becomes ready
Jan 18 10:08:26 jonsmirl dhclient: DHCPREQUEST on eth0 to
255.255.255.255 port 67
Jan 18 10:08:31 jonsmirl dhclient: DHCPREQUEST on eth0 to
255.255.255.255 port 67
Jan 18 10:08:38 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 8
Jan 18 10:08:46 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 10
Jan 18 10:08:56 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 11
Jan 18 10:09:07 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 20
Jan 18 10:09:27 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 9
Jan 18 10:09:36 jonsmirl dhclient: DHCPDISCOVER on eth0 to
255.255.255.255 port 67 interval 3
Jan 18 10:09:39 jonsmirl dhclient: No DHCPOFFERS received.

I am using an old switch so I normally get a half duplex connection:
Jan 18 10:14:25 jonsmirl kernel: e1000: eth0: e1000_watchdog_task: NIC
Link is Up 100 Mbps Half Duplex
I don't normally get the NETDEV up and changed notifications.

--
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] e1000 C style badness
  2006-01-18  8:07 Jens Axboe
@ 2006-01-18  8:37 ` Jens Axboe
  2006-01-18 17:53   ` Jon Smirl
  0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2006-01-18  8:37 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: Linux Kernel, Linus Torvalds

Hi,

Seems e1000 has even bigger problems than just C style badness in the
newest -git after the e1000 updates. diff of kernel boot messages from
e1000:

-Intel(R) PRO/1000 Network Driver - version 6.1.16-k2
+Intel(R) PRO/1000 Network Driver - version 6.3.9-k2
 Copyright (c) 1999-2005 Intel Corporation.
 ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 16 (level, low) -> IRQ 169
 PCI: Setting latency timer of device 0000:02:00.0 to 64
+e1000: 0000:02:00.0: e1000_probe: (PCI Express:2.5Gb/s:Width x1)
00:0c:f1:ff:92:b8
 e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection

It sends/receives 5 packages, but the dhcp request never succeeds. If I
down the interface, it oopses (nvidia module is loaded, however it bombs
in the same way without it). If I revert just the e1000 updates, my
system work fine (typing this message from it).

Unable to handle kernel NULL pointer dereference at 0000000000000160 RIP: 
<ffffffff80319d0b>{skb_drop_fraglist+20}
PGD 0 
Oops: 0000 [1] SMP 
CPU 1 
Modules linked in: nvidia
Pid: 2806, comm: dhcpcd Tainted: P      2.6.16-rc1 #15
RIP: 0010:[<ffffffff80319d0b>] <ffffffff80319d0b>{skb_drop_fraglist+20}
RSP: 0018:ffff81003a39fce8  EFLAGS: 00010206
RAX: ffff81003c8dce80 RBX: ffff81003e28ec80 RCX: 0000000000000002
RDX: 0000000000000800 RSI: 000000003e32c012 RDI: 0000000000000160
RBP: ffff81003f33a1c0 R08: 000000000000000f R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffff81003f345000 R14: ffff81003f011000 R15: ffff81003a162610
FS:  00002aff325eab00(0000) GS:ffff81003f2ce440(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000160 CR3: 000000003a617000 CR4: 00000000000006e0
Process dhcpcd (pid: 2806, threadinfo ffff81003a39e000, task ffff81003f22eea0)
Stack: ffff81003e28ec80 ffffffff80319dc4 0000000000000000 ffff81003e28ec80 
       ffff81003f33a1c0 ffffffff80319de2 ffffc20000036000 ffffffff80293534 
       ffff81003a39fd56 ffff81003f011500 
Call Trace: <ffffffff80319dc4>{skb_release_data+136}
       <ffffffff80319de2>{kfree_skbmem+9} <ffffffff80293534>{e1000_clean_rx_ring+137}
       <ffffffff80293696>{e1000_clean_all_rx_rings+32} <ffffffff80297c85>{e1000_down+279}
       <ffffffff802983d5>{e1000_close+21} <ffffffff8031cf11>{dev_close+85}
       <ffffffff8031cd1a>{dev_change_flags+97} <ffffffff80352f1e>{inet_ioctl+0}
       <ffffffff803528d8>{devinet_ioctl+542} <ffffffff80314a86>{sock_ioctl+500}
       <ffffffff80173f45>{do_ioctl+33} <ffffffff801741c9>{vfs_ioctl+570}
       <ffffffff8017421e>{sys_ioctl+60} <ffffffff8010a81e>{system_call+126}

Code: 48 8b 1f 8b 87 ac 00 00 00 ff c8 75 05 0f ae e8 eb 0e f0 ff 
RIP <ffffffff80319d0b>{skb_drop_fraglist+20} RSP <ffff81003a39fce8>
CR2: 0000000000000160
 
-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] e1000 C style badness
@ 2006-01-18  8:07 Jens Axboe
  2006-01-18  8:37 ` Jens Axboe
  0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2006-01-18  8:07 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: Linux Kernel, Linus Torvalds

Hi,

Recent e1000 updates introduced variable declarations after code. Fix
those up again.

Signed-off-by: Jens Axboe <axboe@suse.de>

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index d0a5d16..ca68a04 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2142,9 +2142,11 @@ e1000_leave_82542_rst(struct e1000_adapt
 		e1000_pci_set_mwi(&adapter->hw);
 
 	if(netif_running(netdev)) {
+		struct e1000_rx_ring *ring;
+
 		e1000_configure_rx(adapter);
 		/* No need to loop, because 82542 supports only 1 queue */
-		struct e1000_rx_ring *ring = &adapter->rx_ring[0];
+		ring = &adapter->rx_ring[0];
 		adapter->alloc_rx_buf(adapter, ring, E1000_DESC_UNUSED(ring));
 	}
 }
@@ -3583,8 +3585,8 @@ e1000_clean_rx_irq(struct e1000_adapter 
 	rx_desc = E1000_RX_DESC(*rx_ring, i);
 
 	while(rx_desc->status & E1000_RXD_STAT_DD) {
-		buffer_info = &rx_ring->buffer_info[i];
 		u8 status;
+		buffer_info = &rx_ring->buffer_info[i];
 #ifdef CONFIG_E1000_NAPI
 		if(*work_done >= work_to_do)
 			break;

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2006-01-21 11:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <5wgyi-18w-7@gated-at.bofh.it>
2006-01-18  8:56 ` [PATCH] e1000 C style badness Patrizio Bassi
2006-01-18  9:00   ` Jens Axboe
2006-01-18  9:02   ` Nick Piggin
2006-01-18 11:30     ` Patrizio Bassi
2006-01-18  8:07 Jens Axboe
2006-01-18  8:37 ` Jens Axboe
2006-01-18 17:53   ` Jon Smirl
2006-01-18 18:13     ` Jeff Garzik
2006-01-18 18:20       ` Jens Axboe
2006-01-18 19:10         ` Jesse Brandeburg
2006-01-19  8:02           ` Jens Axboe
2006-01-21  6:52             ` Jesse Brandeburg
2006-01-21 11:42               ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).