Re: [PATCH 03/23] e100: Add debugging code for cb cleaning and csum failures.
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.
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.
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.
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.
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.
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.
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