From owner-freebsd-stable@FreeBSD.ORG Mon Jul 10 20:40:53 2006 Return-Path: X-Original-To: freebsd-stable@freebsd.org Delivered-To: freebsd-stable@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B36C916A4E6; Mon, 10 Jul 2006 20:40:53 +0000 (UTC) (envelope-from atanas@asd.aplus.net) Received: from pro20.abac.com (pro20.abac.com [66.226.64.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id E994743D46; Mon, 10 Jul 2006 20:40:52 +0000 (GMT) (envelope-from atanas@asd.aplus.net) Received: from [216.55.129.5] (asd2.aplus.net [216.55.129.5]) (authenticated bits=0) by pro20.abac.com (8.13.6/8.13.6) with ESMTP id k6AKemVp084464 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 10 Jul 2006 13:40:49 -0700 (PDT) (envelope-from atanas@asd.aplus.net) Message-ID: <44B2BCDB.7050403@asd.aplus.net> Date: Mon, 10 Jul 2006 13:47:23 -0700 From: Atanas User-Agent: Thunderbird 1.5.0.4 (Macintosh/20060516) MIME-Version: 1.0 To: pyunyh@gmail.com References: <44AC6793.2070608@asd.aplus.net> <20060706021444.GA76865@cdnetworks.co.kr> <44AD7297.7080605@asd.aplus.net> <20060707010341.GD82406@cdnetworks.co.kr> <44ADC2ED.4070904@asd.aplus.net> <20060707040838.GE82406@cdnetworks.co.kr> <20060707151640.D51390@fledge.watson.org> <44AEB0CB.5060102@asd.aplus.net> <20060707181750.O1171@ganymede.hub.org> <20060707223609.N60542@fledge.watson.org> <20060708033254.GB87930@cdnetworks.co.kr> In-Reply-To: <20060708033254.GB87930@cdnetworks.co.kr> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: 1.47 (SPF_SOFTFAIL) Cc: Peter Jeremy , Robert Watson , Michael Vince , freebsd-stable@freebsd.org, User Freebsd Subject: Re: em device hangs on ifconfig alias ... X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jul 2006 20:40:53 -0000 Pyun YongHyeon said the following on 7/7/06 8:32 PM: > On Fri, Jul 07, 2006 at 10:38:01PM +0100, Robert Watson wrote: > > > > Yes -- basically, there are two problems: > > > > (1) A little problem, in which an arp announcement is sent before the link > > has > > settled after reset. > > > > (2) A big problem, in which the interface is gratuitously recent requiring > > long settling times. > > > > I'd really like to see a fix to the second of these problems (not resetting > > when an IP is added or removed, resulting in link renegotiation); the first > > one I'm less concerned about, although it would make some amount of sense > > to do an arp announcement when the link goes up. > > > > Ah, I see. Thanks for the insight. > How about the attached patch? > This patch seems to fix both of the issues, or at least this is what I see now: - the card no longer gets reset when adding an alias; - the arp packet gets delivered; - adding 250 aliases takes less than a second; I haven't fully tested whether all 250 IP aliases were accessible (I used non-routable IP addresses), but I suppose so. Also I couldn't stress the patched driver enough to see whether it performs as expected. But in overall it looks good. I guess some more testing might be needed in order to merge the patch into the source tree. Regards, Atanas > > > ------------------------------------------------------------------------ > > Index: if_em.c > =================================================================== > RCS file: /pool/ncvs/src/sys/dev/em/if_em.c,v > retrieving revision 1.116 > diff -u -r1.116 if_em.c > --- if_em.c 6 Jun 2006 08:03:49 -0000 1.116 > +++ if_em.c 8 Jul 2006 03:30:36 -0000 > @@ -67,6 +67,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -692,6 +693,9 @@ > > EM_LOCK_ASSERT(sc); > > + if ((ifp->if_drv_flags & (IFF_DRV_RUNNING|IFF_DRV_OACTIVE)) != > + IFF_DRV_RUNNING) > + return; > if (!sc->link_active) > return; > > @@ -745,6 +749,7 @@ > { > struct em_softc *sc = ifp->if_softc; > struct ifreq *ifr = (struct ifreq *)data; > + struct ifaddr *ifa = (struct ifaddr *)data; > int error = 0; > > if (sc->in_detach) > @@ -752,9 +757,22 @@ > > switch (command) { > case SIOCSIFADDR: > - case SIOCGIFADDR: > - IOCTL_DEBUGOUT("ioctl rcv'd: SIOCxIFADDR (Get/Set Interface Addr)"); > - ether_ioctl(ifp, command, data); > + if (ifa->ifa_addr->sa_family == AF_INET) { > + /* > + * XXX > + * Since resetting hardware takes a very long time > + * we only initialize the hardware only when it is > + * absolutely required. > + */ > + ifp->if_flags |= IFF_UP; > + if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + EM_LOCK(sc); > + em_init_locked(sc); > + EM_UNLOCK(sc); > + } > + arp_ifinit(ifp, ifa); > + } else > + error = ether_ioctl(ifp, command, data); > break; > case SIOCSIFMTU: > { > @@ -802,17 +820,19 @@ > IOCTL_DEBUGOUT("ioctl rcv'd: SIOCSIFFLAGS (Set Interface Flags)"); > EM_LOCK(sc); > if (ifp->if_flags & IFF_UP) { > - if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + if ((ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + if ((ifp->if_flags ^ sc->if_flags) & > + IFF_PROMISC) { > + em_disable_promisc(sc); > + em_set_promisc(sc); > + } > + } else > em_init_locked(sc); > - } > - > - em_disable_promisc(sc); > - em_set_promisc(sc); > } else { > - if (ifp->if_drv_flags & IFF_DRV_RUNNING) { > + if (ifp->if_drv_flags & IFF_DRV_RUNNING) > em_stop(sc); > - } > } > + sc->if_flags = ifp->if_flags; > EM_UNLOCK(sc); > break; > case SIOCADDMULTI: > @@ -878,8 +898,8 @@ > break; > } > default: > - IOCTL_DEBUGOUT1("ioctl received: UNKNOWN (0x%x)", (int)command); > - error = EINVAL; > + error = ether_ioctl(ifp, command, data); > + break; > } > > return (error); > Index: if_em.h > =================================================================== > RCS file: /pool/ncvs/src/sys/dev/em/if_em.h,v > retrieving revision 1.44 > diff -u -r1.44 if_em.h > --- if_em.h 15 Feb 2006 08:39:50 -0000 1.44 > +++ if_em.h 8 Jul 2006 03:30:43 -0000 > @@ -259,6 +259,7 @@ > struct callout timer; > struct callout tx_fifo_timer; > int io_rid; > + int if_flags; > struct mtx mtx; > int em_insert_vlan_header; > struct task link_task; > > > ------------------------------------------------------------------------ > > Index: if_em.c > =================================================================== > RCS file: /pool/ncvs/src/sys/dev/em/if_em.c,v > retrieving revision 1.65.2.16 > diff -u -r1.65.2.16 if_em.c > --- if_em.c 19 May 2006 00:19:57 -0000 1.65.2.16 > +++ if_em.c 8 Jul 2006 03:29:16 -0000 > @@ -657,8 +657,9 @@ > > mtx_assert(&adapter->mtx, MA_OWNED); > > - if (!adapter->link_active) > - return; > + if ((ifp->if_drv_flags & (IFF_DRV_RUNNING|IFF_DRV_OACTIVE)) != > + IFF_DRV_RUNNING) > + return; > > while (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) { > > @@ -714,15 +715,29 @@ > { > struct ifreq *ifr = (struct ifreq *) data; > struct adapter * adapter = ifp->if_softc; > + struct ifaddr *ifa = (struct ifaddr *)data; > int error = 0; > > if (adapter->in_detach) return(error); > > switch (command) { > case SIOCSIFADDR: > - case SIOCGIFADDR: > - IOCTL_DEBUGOUT("ioctl rcv'd: SIOCxIFADDR (Get/Set Interface Addr)"); > - ether_ioctl(ifp, command, data); > + if (ifa->ifa_addr->sa_family == AF_INET) { > + /* > + * XXX > + * Since resetting hardware takes a very long time > + * we only initialize the hardware only when it is > + * absolutely required. > + */ > + ifp->if_flags |= IFF_UP; > + if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + EM_LOCK(adapter); > + em_init_locked(adapter); > + EM_UNLOCK(adapter); > + } > + arp_ifinit(ifp, ifa); > + } else > + error = ether_ioctl(ifp, command, data); > break; > case SIOCSIFMTU: > { > @@ -760,12 +775,14 @@ > IOCTL_DEBUGOUT("ioctl rcv'd: SIOCSIFFLAGS (Set Interface Flags)"); > EM_LOCK(adapter); > if (ifp->if_flags & IFF_UP) { > - if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + if ((ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + if ((ifp->if_flags ^ adapter->if_flags) & > + IFF_PROMISC) { > + em_disable_promisc(adapter); > + em_set_promisc(adapter); > + } > + } else > em_init_locked(adapter); > - } > - > - em_disable_promisc(adapter); > - em_set_promisc(adapter); > } else { > if (ifp->if_drv_flags & IFF_DRV_RUNNING) { > em_stop(adapter); > @@ -835,8 +852,8 @@ > break; > } > default: > - IOCTL_DEBUGOUT1("ioctl received: UNKNOWN (0x%x)", (int)command); > - error = EINVAL; > + error = ether_ioctl(ifp, command, data); > + break; > } > > return(error); > Index: if_em.h > =================================================================== > RCS file: /pool/ncvs/src/sys/dev/em/if_em.h,v > retrieving revision 1.32.2.2 > diff -u -r1.32.2.2 if_em.h > --- if_em.h 25 Nov 2005 14:11:59 -0000 1.32.2.2 > +++ if_em.h 8 Jul 2006 03:29:25 -0000 > @@ -65,6 +65,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -331,6 +332,7 @@ > struct callout timer; > struct callout tx_fifo_timer; > int io_rid; > + int if_flags; > u_int8_t unit; > struct mtx mtx; > int em_insert_vlan_header; > > > ------------------------------------------------------------------------ > > _______________________________________________ > freebsd-stable@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-stable > To unsubscribe, send any mail to "freebsd-stable-unsubscribe@freebsd.org"