From owner-freebsd-stable@FreeBSD.ORG Sat Jul 8 03:29:05 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 830A516A4DD for ; Sat, 8 Jul 2006 03:29:05 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from nz-out-0102.google.com (nz-out-0102.google.com [64.233.162.193]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0C4E643D53 for ; Sat, 8 Jul 2006 03:29:03 +0000 (GMT) (envelope-from pyunyh@gmail.com) Received: by nz-out-0102.google.com with SMTP id 12so221916nzp for ; Fri, 07 Jul 2006 20:29:03 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:date:from:to:cc:subject:message-id:reply-to:references:mime-version:content-type:content-disposition:in-reply-to:user-agent; b=fCYEBmqBsVrDfrN9v3osRYA8KPNsrA1RIJCyEfCgQUX4fxEyTk0unt28dqA+ZM948nReEM/a+RpqpV4CQj1H16iONZbbY5fAJuY3byYHAistMCVb6uuZrOK06AlbBIvcJ7GchjILbdXggwh3RJYTEh7Po98SQb0xW2qCPDrGnaI= Received: by 10.36.19.19 with SMTP id 19mr3237197nzs; Fri, 07 Jul 2006 20:29:03 -0700 (PDT) Received: from michelle.cdnetworks.co.kr ( [211.53.35.84]) by mx.gmail.com with ESMTP id 18sm287391nzo.2006.07.07.20.28.59; Fri, 07 Jul 2006 20:29:02 -0700 (PDT) Received: from michelle.cdnetworks.co.kr (localhost.cdnetworks.co.kr [127.0.0.1]) by michelle.cdnetworks.co.kr (8.13.5/8.13.5) with ESMTP id k683X04N089041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 8 Jul 2006 12:33:00 +0900 (KST) (envelope-from pyunyh@gmail.com) Received: (from yongari@localhost) by michelle.cdnetworks.co.kr (8.13.5/8.13.5/Submit) id k683Wtbh089027; Sat, 8 Jul 2006 12:32:55 +0900 (KST) (envelope-from pyunyh@gmail.com) Date: Sat, 8 Jul 2006 12:32:55 +0900 From: Pyun YongHyeon To: Robert Watson Message-ID: <20060708033254.GB87930@cdnetworks.co.kr> 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> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="k1lZvvs/B4yU6o8G" Content-Disposition: inline In-Reply-To: <20060707223609.N60542@fledge.watson.org> User-Agent: Mutt/1.4.2.1i Cc: Peter Jeremy , Atanas , freebsd-stable@FreeBSD.org, Michael Vince , User Freebsd Subject: Re: em device hangs on ifconfig alias ... X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Jul 2006 03:29:05 -0000 --k1lZvvs/B4yU6o8G Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jul 07, 2006 at 10:38:01PM +0100, Robert Watson wrote: > > On Fri, 7 Jul 2006, User Freebsd wrote: > > >>I think that I have patched, built and loaded the em(4) kernel module > >>correctly. After applying the patch there were no rejects, before > >>building the module I intentionally appended " (patched)" to its version > >>string in if_em.c, and could see that in dmesg every time I loaded the > >>module: em1: >>(patched)> > > > >Is it possible that we're going at this issue backwards? It isn't the > >lack of ARP packet going out that is causing the problems with moving IPs, > >but that delay that we're seeing when aliasing a new IP on the stack? The > >ARP packet *is* being attempted, but is timing out before the re-init is > >completing? > > 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? -- Regards, Pyun YongHyeon --k1lZvvs/B4yU6o8G Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="em.arp.HEAD.patch2" 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; --k1lZvvs/B4yU6o8G Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="em.arp.REL_6.patch2" 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; --k1lZvvs/B4yU6o8G--