Date: Mon, 31 Jul 2006 20:21:19 -0300 (ADT) From: User Freebsd <freebsd@hub.org> To: Atanas <atanas@asd.aplus.net> Cc: pyunyh@gmail.com, Peter Jeremy <peterjeremy@optushome.com.au>, Robert Watson <rwatson@freebsd.org>, Michael Vince <mv@thebeastie.org>, freebsd-stable@freebsd.org Subject: Re: em device hangs on ifconfig alias ... Message-ID: <20060731202110.F27679@ganymede.hub.org> In-Reply-To: <44B2BCDB.7050403@asd.aplus.net> 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> <44B2BCDB.7050403@asd.aplus.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Any status on this patch being merged in?
On Mon, 10 Jul 2006, Atanas wrote:
> 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 <netinet/in_systm.h>
>> #include <netinet/in.h>
>> +#include <netinet/if_ether.h>
>> #include <netinet/ip.h>
>> #include <netinet/tcp.h>
>> #include <netinet/udp.h>
>> @@ -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 <netinet/in_systm.h>
>> #include <netinet/in.h>
>> +#include <netinet/if_ether.h>
>> #include <netinet/ip.h>
>> #include <netinet/tcp.h>
>> #include <netinet/udp.h>
>> @@ -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"
>
>
----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email . scrappy@hub.org MSN . scrappy@hub.org
Yahoo . yscrappy Skype: hub.org ICQ . 7615664
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060731202110.F27679>