Re: port-sparc64/46260: gem0 driver fails to recover after RX overflow

[Julian Coleman <jdc%coris.org.uk@localhost>]

The following reply was made to PR port-sparc64/46260; it has been noted by 
GNATS.

From: Julian Coleman <jdc%coris.org.uk@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: port-sparc64/46260: gem0 driver fails to recover after RX overflow
Date: Wed, 11 Apr 2012 19:56:00 +0100

 Hi,
 
 > Scratch that diff, here is one which works, but which ends up
 > resetting the interface Quite Often, despite the state register
 > indicating it's not in overflow mode -- it prints
 > 
 >   gem0: rx_watchdog: not in overflow state: 0x10400
 > 
 > Only once in my testing did I see
 > 
 >   gem0: rx_watchdog: rd pointer != saved
 > 
 > occur, but it *did* occur.
 
 I think that this approach (watchdog) is the way to go.  A few comments:
 
 > +                    aprint_error_dev(sc->sc_dev,
 > +                        "receive error: RX no buffer space\n");
 
 I wonder if we should print out these extra diagnostics (we don't do it
 in other drivers).  I think that we would be better off by updating extra
 counters, and using them as per the suggestions in "Using event counters
 with network interfaces, is there a reason they're all ifdefed out of
 mainline use?" thread, starting at:
 
   http://mail-index.NetBSD.org/tech-kern/2011/12/10/msg012122.html
 
 I think that the reason that you're seeing the extra resets is the difference
 between checking GEM_MAC_RX_OVERFLOW in gem_intr():
 
 >              if (rxstat & GEM_MAC_RX_OVERFLOW) {
 >                      ifp->if_ierrors++;
 > +                    aprint_error_dev(sc->sc_dev,
 > +                        "receive error: RX overflow\n");
 >                      gem_reset_rxdma(sc);
 
 but checking GEM_MAC_STATE_OVERFLOW in gem_rx_watchdog():
 
 > +    if ((state & GEM_MAC_STATE_OVERFLOW) == GEM_MAC_STATE_OVERFLOW &&
 
 .  However, I'm not sure if we should be checking GEM_MAC_STATE_OVERFLOW and
 GEM_MAC_RX_OVERFLOW in gem_intr().  Maybe we can check GEM_MAC_STATE_OVERFLOW
 if GEM_MAC_RX_OVERFLOW is set and fire the callout only then.  Alternatively,
 we might only need to check GEM_MAC_STATE_OVERFLOW (and not bother with
 GEM_MAC_RX_OVERFLOW at all).
 
 Thanks,
 
 J
 
 -- 
   My other computer also runs NetBSD    /        Sailing at Newbiggin
         http://www.netbsd.org/        /   http://www.newbigginsailingclub.org/