Re: em device hangs on ifconfig alias ...

Mon, 10 Jul 2006 13:41:23 -0700 

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 "[EMAIL PROTECTED]"


_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to