Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Auke Kok

Jeff Garzik wrote:

Kok, Auke wrote:
Refine cb cleaning debug printout and print out all cleaned cbs' 
status. Add

debug flag for EEPROM csum failures that were overridden by the user.

Signed-off-by: Jesse Brandeburg <[EMAIL PROTECTED]>
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>


ACK patch, NAK description:  tainting is far more than a debug flag.


For now I'll drop the taint flag from this patch until a better one comes up.

Cheers,

Auke
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Auke Kok

Dave Jones wrote:

On Tue, Sep 19, 2006 at 05:40:34PM -0400, Jeff Garzik wrote:
 > Dave Jones wrote:
 > > On Tue, Sep 19, 2006 at 10:28:38AM -0700, Kok, Auke wrote:
 > >  > +  add_taint(TAINT_MACHINE_CHECK);
 > > 
 > > I object to this flag being abused this way.

 > > A corrupt EEPROM on a network card has _nothing_ to do with
 > > a CPU machine check exception.
 > 
 > Fair enough.  Better suggestions?
 > 
 > I think it's fair to set _some_ taint flag, perhaps a new one, on a 
 > known corrupted firmware.  But if others disagree, I'll follow the 
 > consensus here.


I don't object to a new flag, but overloading an existing flag that has
established meaning just seems wrong to me.

Question is how many more types of random hardware failures are there
that we'd like to do similar things for ?
Perhaps a catch-all "H"ardware failure flag for assorted brokenness
would be better than a proliferation of flags?


fine with me. I don't have a strong preference for any specific flag, but 
"H"ardware sounds reasonable.


Auke
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Dave Jones
On Tue, Sep 19, 2006 at 05:40:34PM -0400, Jeff Garzik wrote:
 > Dave Jones wrote:
 > > On Tue, Sep 19, 2006 at 10:28:38AM -0700, Kok, Auke wrote:
 > >  > +   add_taint(TAINT_MACHINE_CHECK);
 > > 
 > > I object to this flag being abused this way.
 > > A corrupt EEPROM on a network card has _nothing_ to do with
 > > a CPU machine check exception.
 > 
 > Fair enough.  Better suggestions?
 > 
 > I think it's fair to set _some_ taint flag, perhaps a new one, on a 
 > known corrupted firmware.  But if others disagree, I'll follow the 
 > consensus here.

I don't object to a new flag, but overloading an existing flag that has
established meaning just seems wrong to me.

Question is how many more types of random hardware failures are there
that we'd like to do similar things for ?
Perhaps a catch-all "H"ardware failure flag for assorted brokenness
would be better than a proliferation of flags?

Dave

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Jeff Garzik

Dave Jones wrote:

On Tue, Sep 19, 2006 at 10:28:38AM -0700, Kok, Auke wrote:
 > 
 > Refine cb cleaning debug printout and print out all cleaned cbs' status. Add

 > debug flag for EEPROM csum failures that were overridden by the user.
 > 
 > Signed-off-by: Jesse Brandeburg <[EMAIL PROTECTED]>

 > Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
 > ---
 > 
 >  drivers/net/e100.c |9 ++---

 >  1 files changed, 6 insertions(+), 3 deletions(-)
 > 
 > diff --git a/drivers/net/e100.c b/drivers/net/e100.c

 > index ab0868c..ae93c62 100644
 > --- a/drivers/net/e100.c
 > +++ b/drivers/net/e100.c
 > @@ -761,6 +761,8 @@ static int e100_eeprom_load(struct nic *
 >   DPRINTK(PROBE, ERR, "EEPROM corrupted\n");
 >   if (!eeprom_bad_csum_allow)
 >   return -EAGAIN;
 > + else
 > + add_taint(TAINT_MACHINE_CHECK);

I object to this flag being abused this way.
A corrupt EEPROM on a network card has _nothing_ to do with
a CPU machine check exception.


Fair enough.  Better suggestions?

I think it's fair to set _some_ taint flag, perhaps a new one, on a 
known corrupted firmware.  But if others disagree, I'll follow the 
consensus here.


Jeff, not having a strong opinion in this case



-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Dave Jones
On Tue, Sep 19, 2006 at 10:28:38AM -0700, Kok, Auke wrote:
 > 
 > Refine cb cleaning debug printout and print out all cleaned cbs' status. Add
 > debug flag for EEPROM csum failures that were overridden by the user.
 > 
 > Signed-off-by: Jesse Brandeburg <[EMAIL PROTECTED]>
 > Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
 > ---
 > 
 >  drivers/net/e100.c |9 ++---
 >  1 files changed, 6 insertions(+), 3 deletions(-)
 > 
 > diff --git a/drivers/net/e100.c b/drivers/net/e100.c
 > index ab0868c..ae93c62 100644
 > --- a/drivers/net/e100.c
 > +++ b/drivers/net/e100.c
 > @@ -761,6 +761,8 @@ static int e100_eeprom_load(struct nic *
 >  DPRINTK(PROBE, ERR, "EEPROM corrupted\n");
 >  if (!eeprom_bad_csum_allow)
 >  return -EAGAIN;
 > +else
 > +add_taint(TAINT_MACHINE_CHECK);

I object to this flag being abused this way.
A corrupt EEPROM on a network card has _nothing_ to do with
a CPU machine check exception.

Dave

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Jeff Garzik

Kok, Auke wrote:

Refine cb cleaning debug printout and print out all cleaned cbs' status. Add
debug flag for EEPROM csum failures that were overridden by the user.

Signed-off-by: Jesse Brandeburg <[EMAIL PROTECTED]>
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>


ACK patch, NAK description:  tainting is far more than a debug flag.


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.

2006-09-19 Thread Kok, Auke

Refine cb cleaning debug printout and print out all cleaned cbs' status. Add
debug flag for EEPROM csum failures that were overridden by the user.

Signed-off-by: Jesse Brandeburg <[EMAIL PROTECTED]>
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
---

 drivers/net/e100.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index ab0868c..ae93c62 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -761,6 +761,8 @@ static int e100_eeprom_load(struct nic *
DPRINTK(PROBE, ERR, "EEPROM corrupted\n");
if (!eeprom_bad_csum_allow)
return -EAGAIN;
+   else
+   add_taint(TAINT_MACHINE_CHECK);
}
 
return 0;
@@ -1657,13 +1659,14 @@ static int e100_tx_clean(struct nic *nic
 
spin_lock(&nic->cb_lock);
 
-   DPRINTK(TX_DONE, DEBUG, "cb->status = 0x%04X\n",
-   nic->cb_to_clean->status);
-
/* Clean CBs marked complete */
for(cb = nic->cb_to_clean;
cb->status & cpu_to_le16(cb_complete);
cb = nic->cb_to_clean = cb->next) {
+   DPRINTK(TX_DONE, DEBUG, "cb[%d]->status = 0x%04X\n",
+   (int)(((void*)cb - (void*)nic->cbs)/sizeof(struct cb)),
+   cb->status);
+
if(likely(cb->skb != NULL)) {
nic->net_stats.tx_packets++;
nic->net_stats.tx_bytes += cb->skb->len;



---
Auke Kok <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html