Date: Sat, 8 Jul 2006 12:32:55 +0900 From: Pyun YongHyeon <pyunyh@gmail.com> To: Robert Watson <rwatson@FreeBSD.org> Cc: Peter Jeremy <peterjeremy@optushome.com.au>, Atanas <atanas@asd.aplus.net>, freebsd-stable@FreeBSD.org, Michael Vince <mv@thebeastie.org>, User Freebsd <freebsd@hub.org> Subject: Re: em device hangs on ifconfig alias ... Message-ID: <20060708033254.GB87930@cdnetworks.co.kr> In-Reply-To: <20060707223609.N60542@fledge.watson.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--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: <Intel(R) PRO/1000 Network Connection Version - 3.2.18
> >>(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 <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;
--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 <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;
--k1lZvvs/B4yU6o8G--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060708033254.GB87930>