Re: CVS commit: src/sys/dev/ic

[christos%zoulas.com@localhost (Christos Zoulas)]

On Oct 28,  7:08am, thorpej%shagadelic.org@localhost (Jason Thorpe) wrote:
-- Subject: Re: CVS commit: src/sys/dev/ic

| 
| On Oct 28, 2005, at 6:11 AM, Christos Zoulas wrote:
| 
| > This ether_ioctl handling resets the chip unnecessarily which
| > results in carrier loss. So you try to use tcpdump and you end up
| > stuck for 30 seconds or more while the cisco switch recomputes its
| > spanning tree. I am not sure this is the case for the gem driver,
| > but for other drivers it surely is.
| 
| Well, if you don't need to reset the chip to reprogram the address  
| filter, then just change the ENETRESET handling in gem_ioctl().  The  
| tlp driver, for example, does not reset the chip on ENETRESET, just  
| reprograms the receive filter.  I suggest you make a similar change  
| to the gem driver.  This is safe, because for Ethernet, ENETRESET is  
| only used to indicate that the multicast list has changed.

While this would be nice, this is not the case right now as the following
code shows taken from ether_ioctl:

        case SIOCSIFFLAGS:
                if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) == IFF_RUNNING) {
                        /*
                         * If interface is marked down and it is running,
                         * then stop and disable it.
                         */
                        (*ifp->if_stop)(ifp, 1);
                } else if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) == IFF_UP) {
                        /*
                         * If interface is marked up and it is stopped, then
                         * start it.
                         */
                        error = (*ifp->if_init)(ifp);
                } else if ((ifp->if_flags & IFF_UP) != 0) {
                        /*
                         * Reset the interface to pick up changes in any other
                         * flags that affect the hardware state.
                         */
                        error = (*ifp->if_init)(ifp);
                }
                break;

So if the interface is up, and we make a transition between non promiscuous
and promiscuous mode [thus changing flags only when the interface is up]
we call if_init, which resets the chip. What gives?

christos