Re: em(4) + ALTQ broken

2010-02-11 Thread Nick Rogers
Anyone else get a chance to review this?

On Fri, Feb 5, 2010 at 8:44 PM, Nick Rogers ncrog...@gmail.com wrote:

 I applied drbr_altq.diff to the e1000 driver (sys/dev/e1000) from HEAD on
 top of 8.0-RELEASE kernel sources. It appears to have fixed the immediate
 problem where queues simply don't work on em interfaces. Thanks a bunch.

 I suppose further review and testing by others would be greatly appreciated
 from my point of view. I am trying to decide on a relatively stable 8.0
 kernel with working em(4) + ALTQ to put into production on 100 or so
 installations. Are you guys more comfortable with the HEAD sys/dev/e1000 +
 this patch on top of 8.0-RELEASE, or e1000 from 7.2 on top of 8.0-RELEASE?
 So far I am having good luck with the later. Thanks again for your
 contributions!


 On Thu, Feb 4, 2010 at 6:51 PM, Max Laier m...@love2party.net wrote:

 Okay ... attached is a patch to fix this for em(4) (and lay the groundwork
 to
 fix it for other drbr_* consumer as well).  I have tested it in
 VirtualBox,
 but don't have real hardware to check for non-ALTQ performance or other
 regressions.

 Test, comments and review appreciated.

 --
   Max



___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-05 Thread Nick Rogers
I applied drbr_altq.diff to the e1000 driver (sys/dev/e1000) from HEAD on
top of 8.0-RELEASE kernel sources. It appears to have fixed the immediate
problem where queues simply don't work on em interfaces. Thanks a bunch.

I suppose further review and testing by others would be greatly appreciated
from my point of view. I am trying to decide on a relatively stable 8.0
kernel with working em(4) + ALTQ to put into production on 100 or so
installations. Are you guys more comfortable with the HEAD sys/dev/e1000 +
this patch on top of 8.0-RELEASE, or e1000 from 7.2 on top of 8.0-RELEASE?
So far I am having good luck with the later. Thanks again for your
contributions!

On Thu, Feb 4, 2010 at 6:51 PM, Max Laier m...@love2party.net wrote:

 Okay ... attached is a patch to fix this for em(4) (and lay the groundwork
 to
 fix it for other drbr_* consumer as well).  I have tested it in VirtualBox,
 but don't have real hardware to check for non-ALTQ performance or other
 regressions.

 Test, comments and review appreciated.

 --
   Max

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-04 Thread Max Laier
Okay ... attached is a patch to fix this for em(4) (and lay the groundwork to 
fix it for other drbr_* consumer as well).  I have tested it in VirtualBox, 
but don't have real hardware to check for non-ALTQ performance or other 
regressions.

Test, comments and review appreciated.

--
  Max
Index: sys/dev/e1000/if_em.c
===
--- sys/dev/e1000/if_em.c	(revision 203362)
+++ sys/dev/e1000/if_em.c	(working copy)
@@ -943,7 +943,7 @@
 {
 	struct adapter	*adapter = ifp-if_softc;
 	struct mbuf	*next;
-	int error = E1000_SUCCESS;
+	int ret, error = E1000_SUCCESS;
 
 	EM_TX_LOCK_ASSERT(adapter);
 	/* To allow being called from a tasklet */
@@ -955,28 +955,34 @@
 	|| (!adapter-link_active)) {
 		error = drbr_enqueue(ifp, adapter-br, m);
 		return (error);
-	} else if (drbr_empty(ifp, adapter-br) 
-	(adapter-num_tx_desc_avail  EM_TX_OP_THRESHOLD)) {
-		if ((error = em_xmit(adapter, m)) != 0) {
-			if (m)
-error = drbr_enqueue(ifp, adapter-br, m);
+	}
+	ret = drbr_maybe_enqueue(ifp, adapter-br, m);
+	if (ret == 0) {
+		if (adapter-num_tx_desc_avail  EM_TX_OP_THRESHOLD) {
+			if ((error = em_xmit(adapter, m)) != 0) {
+if (m)
+	error = drbr_enqueue(ifp, adapter-br,
+	m);
+return (error);
+			} else {
+/*
+ * We've bypassed the buf ring so we need to
+ * update ifp directly
+ */
+drbr_stats_update(ifp, m-m_pkthdr.len,
+m-m_flags);
+/*
+** Send a copy of the frame to the BPF
+** listener and set the watchdog on.
+*/
+ETHER_BPF_MTAP(ifp, m);
+adapter-watchdog_check = TRUE;
+			}
+		} else if ((error = drbr_enqueue(ifp, adapter-br, m)) != 0)
 			return (error);
-		} else {
-			/*
-			 * We've bypassed the buf ring so we need to update
-			 * ifp directly
-			 */
-			drbr_stats_update(ifp, m-m_pkthdr.len, m-m_flags);
-			/*
-			** Send a copy of the frame to the BPF
-			** listener and set the watchdog on.
-			*/
-			ETHER_BPF_MTAP(ifp, m);
-			adapter-watchdog_check = TRUE;
-		}
-	} else if ((error = drbr_enqueue(ifp, adapter-br, m)) != 0)
-		return (error);
-	
+	} else if (ret  0)
+		return (-ret);
+
 process:
 	if (drbr_empty(ifp, adapter-br))
 		return(error);
@@ -989,7 +995,7 @@
 break;
 if ((error = em_xmit(adapter, next)) != 0) {
 			if (next != NULL)
-error = drbr_enqueue(ifp, adapter-br, next);
+error = drbr_requeue(ifp, adapter-br, next);
 break;
 		}
 		drbr_stats_update(ifp, next-m_pkthdr.len, next-m_flags);
Index: sys/net/if_var.h
===
--- sys/net/if_var.h	(revision 203362)
+++ sys/net/if_var.h	(working copy)
@@ -597,18 +597,26 @@
 	return (error);
 }
 
+static __inline int
+drbr_requeue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+#ifdef ALTQ
+	if (ALTQ_IS_ENABLED(ifp-if_snd)) {
+		IFQ_DRV_PREPEND(ifp-if_snd, m);
+		return (0);
+	}
+#endif
+	return drbr_enqueue(ifp, br, m);
+}
+
 static __inline void
 drbr_flush(struct ifnet *ifp, struct buf_ring *br)
 {
 	struct mbuf *m;
 
 #ifdef ALTQ
-	if (ifp != NULL  ALTQ_IS_ENABLED(ifp-if_snd)) {
-		while (!IFQ_IS_EMPTY(ifp-if_snd)) {
-			IFQ_DRV_DEQUEUE(ifp-if_snd, m);
-			m_freem(m);
-		}
-	}
+	if (ifp != NULL  ALTQ_IS_ENABLED(ifp-if_snd))
+		IFQ_DRV_PURGE(ifp-if_snd);
 #endif	
 	while ((m = buf_ring_dequeue_sc(br)) != NULL)
 		m_freem(m);
@@ -642,11 +650,12 @@
 {
 	struct mbuf *m;
 #ifdef ALTQ
-	/*
-	 * XXX need to evaluate / requeue 
-	 */
 	if (ALTQ_IS_ENABLED(ifp-if_snd)) {	
 		IFQ_DRV_DEQUEUE(ifp-if_snd, m);
+		if (m != NULL  func(m, arg) == 0) {
+			IFQ_DRV_PREPEND(ifp-if_snd, m);
+			return (NULL);
+		}
 		return (m);
 	}
 #endif
@@ -668,6 +677,24 @@
 }
 
 static __inline int
+drbr_maybe_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
+{
+	int error;
+
+#ifdef ALTQ
+	if (ALTQ_IS_ENABLED(ifp-if_snd)) {
+		IFQ_ENQUEUE(ifp-if_snd, m, error);
+		return error ? -error : 1;
+	}
+#endif
+	if (drbr_empty(ifp, br))
+		return 0;
+
+	error = drbr_enqueue(ifp, br, m);
+	return error ? -error : 1;
+}
+
+static __inline int
 drbr_inuse(struct ifnet *ifp, struct buf_ring *br)
 {
 #ifdef ALTQ
diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index 7a2dbad..94ad229 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -1020,7 +1020,7 @@ em_mq_start_locked(struct ifnet *ifp, struct mbuf *m)
 {
 	struct adapter	*adapter = ifp-if_softc;
 	struct mbuf	*next;
-	int error = E1000_SUCCESS;
+	int ret, error = E1000_SUCCESS;
 
 	EM_TX_LOCK_ASSERT(adapter);
 	/* To allow being called from a tasklet */
@@ -1032,28 +1032,34 @@ em_mq_start_locked(struct ifnet *ifp, struct mbuf *m)
 	|| (!adapter-link_active)) {
 		error = drbr_enqueue(ifp, adapter-br, m);
 		return (error);
-	} else if (drbr_empty(ifp, adapter-br) 
-	(adapter-num_tx_desc_avail  EM_TX_OP_THRESHOLD)) {
-		if ((error = em_xmit(adapter, m)) != 0) {
-			if (m != NULL)
-error = 

Re: em(4) + ALTQ broken

2010-02-02 Thread Nick Rogers
 I guess the problem comes from multi-queue support. The drbr
 interface is implemented with inline function so em(4)/igb(4) may
 have to define ALTQ to the header. I have not tested the patch(no
 time at this moment) but would you give it try?

 I tried the patch and it did not work.
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Pyun YongHyeon
On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
  I guess the problem comes from multi-queue support. The drbr
  interface is implemented with inline function so em(4)/igb(4) may
  have to define ALTQ to the header. I have not tested the patch(no
  time at this moment) but would you give it try?
 
  I tried the patch and it did not work.

You rebuilt kernel, right? Rebuilding kernel module has no effect.
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Nick Rogers
On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com wrote:

 On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
   I guess the problem comes from multi-queue support. The drbr
   interface is implemented with inline function so em(4)/igb(4) may
   have to define ALTQ to the header. I have not tested the patch(no
   time at this moment) but would you give it try?
  
   I tried the patch and it did not work.

 You rebuilt kernel, right? Rebuilding kernel module has no effect.

Yes I rebuilt the kernel itself and replaced the one on my test system.
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Jack Vogel
So apparently this thing needs no special knowledge in the driver, yet
something in
the new code breaks it, can someone explain tersely how the altq app
actually
pokes or hooks up to the driver? I am not clear about that and I suspect
if I was
this would all be clearer.

Jack


On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com wrote:

 On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
   I guess the problem comes from multi-queue support. The drbr
   interface is implemented with inline function so em(4)/igb(4) may
   have to define ALTQ to the header. I have not tested the patch(no
   time at this moment) but would you give it try?
  
   I tried the patch and it did not work.

 You rebuilt kernel, right? Rebuilding kernel module has no effect.

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Pyun YongHyeon
On Tue, Feb 02, 2010 at 09:47:17AM -0800, Nick Rogers wrote:
 On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com wrote:
 
  On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
I guess the problem comes from multi-queue support. The drbr
interface is implemented with inline function so em(4)/igb(4) may
have to define ALTQ to the header. I have not tested the patch(no
time at this moment) but would you give it try?
   
I tried the patch and it did not work.
 
  You rebuilt kernel, right? Rebuilding kernel module has no effect.
 
 Yes I rebuilt the kernel itself and replaced the one on my test system.

Hmm, I have to find time to experiment this.
Thank you for testing!
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Max Laier
On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
 So apparently this thing needs no special knowledge in the driver, yet
 something in
 the new code breaks it, can someone explain tersely how the altq app
 actually
 pokes or hooks up to the driver? I am not clear about that and I
  suspect if I was
 this would all be clearer.

The whole story is in

  man 9 altq

long story short, as long as you consistently use the IFQ_* macros to manage 
the interface queue, things should just work.  if_var.h used to conditionally 
define these macros to avoid ALTQ overhead when the kernel is built without 
ALTQ.  This has changed a long time ago and should not make any difference 
anymore.

I can't figure out who the OP is, but could you make sure that the includes 
that are used to built the kernel are up to date?  You are building with the 
buildkernel target and not the old way, right?  Also, if you build just the 
module, the build might pick up the includes from /usr/include instead of 
src/sys ...
 
 Jack
 
 On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com wrote:
  On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
I guess the problem comes from multi-queue support. The drbr
interface is implemented with inline function so em(4)/igb(4) may
have to define ALTQ to the header. I have not tested the patch(no
time at this moment) but would you give it try?
   
I tried the patch and it did not work.
 
  You rebuilt kernel, right? Rebuilding kernel module has no effect.
 
 ___
 freebsd-stable@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/freebsd-stable
 To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org
 
 
 !DSPAM:4b686584144321871135632!
 
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Jack Vogel
Thanks Max, yes, i've done some digging myself and now see how things
work, the rubber meets the road in the defines in if_var.h.

And what it does is effectively short circuit Kip Macy's multiqueue code
in favor of the old method.

Right now I can see two possibilities, either the defines are not set in
the build, OR there is something wrong in the logic of the short circuit
approach in Kip's code.

A question might be if ANY driver that is usinig TX Multiqueue has been
successfully used with ALTQ?

Jack


On Tue, Feb 2, 2010 at 12:37 PM, Max Laier m...@love2party.net wrote:

 On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
  So apparently this thing needs no special knowledge in the driver, yet
  something in
  the new code breaks it, can someone explain tersely how the altq app
  actually
  pokes or hooks up to the driver? I am not clear about that and I
   suspect if I was
  this would all be clearer.

 The whole story is in

  man 9 altq

 long story short, as long as you consistently use the IFQ_* macros to
 manage
 the interface queue, things should just work.  if_var.h used to
 conditionally
 define these macros to avoid ALTQ overhead when the kernel is built without
 ALTQ.  This has changed a long time ago and should not make any difference
 anymore.

 I can't figure out who the OP is, but could you make sure that the includes
 that are used to built the kernel are up to date?  You are building with
 the
 buildkernel target and not the old way, right?  Also, if you build just
 the
 module, the build might pick up the includes from /usr/include instead of
 src/sys ...

  Jack
 
  On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com wrote:
   On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
 I guess the problem comes from multi-queue support. The drbr
 interface is implemented with inline function so em(4)/igb(4) may
 have to define ALTQ to the header. I have not tested the patch(no
 time at this moment) but would you give it try?

 I tried the patch and it did not work.
  
   You rebuilt kernel, right? Rebuilding kernel module has no effect.
 
  ___
  freebsd-stable@freebsd.org mailing list
  http://lists.freebsd.org/mailman/listinfo/freebsd-stable
  To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org
 
 
 
  !DSPAM:4b686584144321871135632!
 

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Jack Vogel
LOL, and I can answer my own question, I just looked and the ONLY
1Gig drivers using multiqueue are mine, so I guess not eh? :)

J.

On Tue, Feb 2, 2010 at 1:39 PM, Jack Vogel jfvo...@gmail.com wrote:

 Thanks Max, yes, i've done some digging myself and now see how things
 work, the rubber meets the road in the defines in if_var.h.

 And what it does is effectively short circuit Kip Macy's multiqueue code
 in favor of the old method.

 Right now I can see two possibilities, either the defines are not set in
 the build, OR there is something wrong in the logic of the short circuit
 approach in Kip's code.

 A question might be if ANY driver that is usinig TX Multiqueue has been
 successfully used with ALTQ?

 Jack



 On Tue, Feb 2, 2010 at 12:37 PM, Max Laier m...@love2party.net wrote:

 On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
  So apparently this thing needs no special knowledge in the driver, yet
  something in
  the new code breaks it, can someone explain tersely how the altq app
  actually
  pokes or hooks up to the driver? I am not clear about that and I
   suspect if I was
  this would all be clearer.

 The whole story is in

  man 9 altq

 long story short, as long as you consistently use the IFQ_* macros to
 manage
 the interface queue, things should just work.  if_var.h used to
 conditionally
 define these macros to avoid ALTQ overhead when the kernel is built
 without
 ALTQ.  This has changed a long time ago and should not make any difference
 anymore.

 I can't figure out who the OP is, but could you make sure that the
 includes
 that are used to built the kernel are up to date?  You are building with
 the
 buildkernel target and not the old way, right?  Also, if you build just
 the
 module, the build might pick up the includes from /usr/include instead of
 src/sys ...

  Jack
 
  On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com
 wrote:
   On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
 I guess the problem comes from multi-queue support. The drbr
 interface is implemented with inline function so em(4)/igb(4) may
 have to define ALTQ to the header. I have not tested the patch(no
 time at this moment) but would you give it try?

 I tried the patch and it did not work.
  
   You rebuilt kernel, right? Rebuilding kernel module has no effect.
 
  ___
  freebsd-stable@freebsd.org mailing list
  http://lists.freebsd.org/mailman/listinfo/freebsd-stable
  To unsubscribe, send any mail to 
 freebsd-stable-unsubscr...@freebsd.org
 
 
  !DSPAM:4b686584144321871135632!
 



___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Pyun YongHyeon
On Tue, Feb 02, 2010 at 01:41:01PM -0800, Jack Vogel wrote:
 LOL, and I can answer my own question, I just looked and the ONLY
 1Gig drivers using multiqueue are mine, so I guess not eh? :)
 

I was wrong. ALTQ is defined in opt_global.h so drbr_ interface
should already see ALTQ. I have to look into drbr_ code.

 J.
 
 On Tue, Feb 2, 2010 at 1:39 PM, Jack Vogel jfvo...@gmail.com wrote:
 
  Thanks Max, yes, i've done some digging myself and now see how things
  work, the rubber meets the road in the defines in if_var.h.
 
  And what it does is effectively short circuit Kip Macy's multiqueue code
  in favor of the old method.
 
  Right now I can see two possibilities, either the defines are not set in
  the build, OR there is something wrong in the logic of the short circuit
  approach in Kip's code.
 
  A question might be if ANY driver that is usinig TX Multiqueue has been
  successfully used with ALTQ?
 
  Jack
 
 
 
  On Tue, Feb 2, 2010 at 12:37 PM, Max Laier m...@love2party.net wrote:
 
  On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
   So apparently this thing needs no special knowledge in the driver, yet
   something in
   the new code breaks it, can someone explain tersely how the altq app
   actually
   pokes or hooks up to the driver? I am not clear about that and I
suspect if I was
   this would all be clearer.
 
  The whole story is in
 
   man 9 altq
 
  long story short, as long as you consistently use the IFQ_* macros to
  manage
  the interface queue, things should just work.  if_var.h used to
  conditionally
  define these macros to avoid ALTQ overhead when the kernel is built
  without
  ALTQ.  This has changed a long time ago and should not make any difference
  anymore.
 
  I can't figure out who the OP is, but could you make sure that the
  includes
  that are used to built the kernel are up to date?  You are building with
  the
  buildkernel target and not the old way, right?  Also, if you build just
  the
  module, the build might pick up the includes from /usr/include instead of
  src/sys ...
 
   Jack
  
   On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com
  wrote:
On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
  I guess the problem comes from multi-queue support. The drbr
  interface is implemented with inline function so em(4)/igb(4) may
  have to define ALTQ to the header. I have not tested the patch(no
  time at this moment) but would you give it try?
 
  I tried the patch and it did not work.
   
You rebuilt kernel, right? Rebuilding kernel module has no effect.
  
   ___
   freebsd-stable@freebsd.org mailing list
   http://lists.freebsd.org/mailman/listinfo/freebsd-stable
   To unsubscribe, send any mail to 
  freebsd-stable-unsubscr...@freebsd.org
  
  
   !DSPAM:4b686584144321871135632!
  
 
 
 
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Jack Vogel
It should never get to the drbr code, look at net/if_var.h, in the inline
definition
of drbr_dequeue() there is an #ifdef ALTQ that will effectively vector if to
use
the old IFQ_DRV_DEQUEUE() method.

I guess I can test the build on a system here, stick some syntax error in
and see if it hits.

Right now the inserted code looks solid enough to me, so somehow I think
its not being defined.

Jack


On Tue, Feb 2, 2010 at 2:38 PM, Pyun YongHyeon pyu...@gmail.com wrote:

 On Tue, Feb 02, 2010 at 01:41:01PM -0800, Jack Vogel wrote:
  LOL, and I can answer my own question, I just looked and the ONLY
  1Gig drivers using multiqueue are mine, so I guess not eh? :)
 

 I was wrong. ALTQ is defined in opt_global.h so drbr_ interface
 should already see ALTQ. I have to look into drbr_ code.

  J.
 
  On Tue, Feb 2, 2010 at 1:39 PM, Jack Vogel jfvo...@gmail.com wrote:
 
   Thanks Max, yes, i've done some digging myself and now see how things
   work, the rubber meets the road in the defines in if_var.h.
  
   And what it does is effectively short circuit Kip Macy's multiqueue
 code
   in favor of the old method.
  
   Right now I can see two possibilities, either the defines are not set
 in
   the build, OR there is something wrong in the logic of the short
 circuit
   approach in Kip's code.
  
   A question might be if ANY driver that is usinig TX Multiqueue has been
   successfully used with ALTQ?
  
   Jack
  
  
  
   On Tue, Feb 2, 2010 at 12:37 PM, Max Laier m...@love2party.net wrote:
  
   On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
So apparently this thing needs no special knowledge in the driver,
 yet
something in
the new code breaks it, can someone explain tersely how the altq app
actually
pokes or hooks up to the driver? I am not clear about that and I
 suspect if I was
this would all be clearer.
  
   The whole story is in
  
man 9 altq
  
   long story short, as long as you consistently use the IFQ_* macros to
   manage
   the interface queue, things should just work.  if_var.h used to
   conditionally
   define these macros to avoid ALTQ overhead when the kernel is built
   without
   ALTQ.  This has changed a long time ago and should not make any
 difference
   anymore.
  
   I can't figure out who the OP is, but could you make sure that the
   includes
   that are used to built the kernel are up to date?  You are building
 with
   the
   buildkernel target and not the old way, right?  Also, if you build
 just
   the
   module, the build might pick up the includes from /usr/include instead
 of
   src/sys ...
  
Jack
   
On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com
   wrote:
 On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
   I guess the problem comes from multi-queue support. The drbr
   interface is implemented with inline function so em(4)/igb(4)
 may
   have to define ALTQ to the header. I have not tested the
 patch(no
   time at this moment) but would you give it try?
  
   I tried the patch and it did not work.

 You rebuilt kernel, right? Rebuilding kernel module has no effect.
   
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to 
   freebsd-stable-unsubscr...@freebsd.org
   
   
!DSPAM:4b686584144321871135632!
   
  
  
  

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-02-02 Thread Jack Vogel
Just teseted, and at least in the kernel build I'm doing its definitely
defining
that code on, hit my syntax error rebuilding em.

Jack


On Tue, Feb 2, 2010 at 2:43 PM, Jack Vogel jfvo...@gmail.com wrote:

 It should never get to the drbr code, look at net/if_var.h, in the inline
 definition
 of drbr_dequeue() there is an #ifdef ALTQ that will effectively vector if
 to use
 the old IFQ_DRV_DEQUEUE() method.

 I guess I can test the build on a system here, stick some syntax error in
 and see if it hits.

 Right now the inserted code looks solid enough to me, so somehow I think
 its not being defined.

 Jack



 On Tue, Feb 2, 2010 at 2:38 PM, Pyun YongHyeon pyu...@gmail.com wrote:

 On Tue, Feb 02, 2010 at 01:41:01PM -0800, Jack Vogel wrote:
  LOL, and I can answer my own question, I just looked and the ONLY
  1Gig drivers using multiqueue are mine, so I guess not eh? :)
 

 I was wrong. ALTQ is defined in opt_global.h so drbr_ interface
 should already see ALTQ. I have to look into drbr_ code.

  J.
 
  On Tue, Feb 2, 2010 at 1:39 PM, Jack Vogel jfvo...@gmail.com wrote:
 
   Thanks Max, yes, i've done some digging myself and now see how things
   work, the rubber meets the road in the defines in if_var.h.
  
   And what it does is effectively short circuit Kip Macy's multiqueue
 code
   in favor of the old method.
  
   Right now I can see two possibilities, either the defines are not set
 in
   the build, OR there is something wrong in the logic of the short
 circuit
   approach in Kip's code.
  
   A question might be if ANY driver that is usinig TX Multiqueue has
 been
   successfully used with ALTQ?
  
   Jack
  
  
  
   On Tue, Feb 2, 2010 at 12:37 PM, Max Laier m...@love2party.net
 wrote:
  
   On Tuesday 02 February 2010 18:48:02 Jack Vogel wrote:
So apparently this thing needs no special knowledge in the driver,
 yet
something in
the new code breaks it, can someone explain tersely how the altq
 app
actually
pokes or hooks up to the driver? I am not clear about that and
 I
 suspect if I was
this would all be clearer.
  
   The whole story is in
  
man 9 altq
  
   long story short, as long as you consistently use the IFQ_* macros to
   manage
   the interface queue, things should just work.  if_var.h used to
   conditionally
   define these macros to avoid ALTQ overhead when the kernel is built
   without
   ALTQ.  This has changed a long time ago and should not make any
 difference
   anymore.
  
   I can't figure out who the OP is, but could you make sure that the
   includes
   that are used to built the kernel are up to date?  You are building
 with
   the
   buildkernel target and not the old way, right?  Also, if you build
 just
   the
   module, the build might pick up the includes from /usr/include
 instead of
   src/sys ...
  
Jack
   
On Tue, Feb 2, 2010 at 9:37 AM, Pyun YongHyeon pyu...@gmail.com
   wrote:
 On Tue, Feb 02, 2010 at 09:30:52AM -0800, Nick Rogers wrote:
   I guess the problem comes from multi-queue support. The drbr
   interface is implemented with inline function so em(4)/igb(4)
 may
   have to define ALTQ to the header. I have not tested the
 patch(no
   time at this moment) but would you give it try?
  
   I tried the patch and it did not work.

 You rebuilt kernel, right? Rebuilding kernel module has no
 effect.
   
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to 
   freebsd-stable-unsubscr...@freebsd.org
   
   
!DSPAM:4b686584144321871135632!
   
  
  
  



___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


em(4) + ALTQ broken

2010-01-31 Thread Nick Rogers
I'm having problems getting PF + ALTQ to work on em(4) interfaces under
8.0-RELEASE. Kernel was rebuilt with the additional options necessary for
ALTQ and what not. Same basic configuration works fine under 7.2-RELEASE.
Basically, the queues create successfully but running a pfctl -vsq shows a
zero packet/byte count for all queues, even the interface's root queues.

This same problem is mentioned in PR kern/138392, and the following forum
thread...
http://forums.freebsd.org/showthread.php?t=6656

em(4)/e1000 driver from CURRENT does not fix the problem. Building an
8.0-RELEASE kernel with the em(4) driver from 7.2-RELEASE fixes the problem
(i.e., replacing sys/dev/e1000 with that from 7.2).

One of the machines im experiencing this on has the following intel
chipset...

[u...@foo ~]$ sysctl dev.em.0
dev.em.0.%desc: Intel(R) PRO/1000 Network Connection 6.9.6
dev.em.0.%driver: em
dev.em.0.%location: slot=0 function=0
dev.em.0.%pnpinfo: vendor=0x8086 device=0x10d3 subvendor=0x15d9
subdevice=0x040d class=0x02
dev.em.0.%parent: pci3
dev.em.0.debug: -1
dev.em.0.stats: -1
dev.em.0.rx_int_delay: 0
dev.em.0.tx_int_delay: 66
dev.em.0.rx_abs_int_delay: 66
dev.em.0.tx_abs_int_delay: 66
dev.em.0.rx_processing_limit: 100

Is this issue receiving any attention? I ask because one of the em(4) driver
contributors mentioned he had not heard of this problem in a recent thread
regarding a different em(4) bug, and this is a pretty serious problem for me
as I have many devices in production that need to be upgraded to 8.0, all
running Intel interfaces and relying on ALTQ.

I appreciate any updates or information and would be happy to test any
patches and/or provide more information. Thanks.
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org


Re: em(4) + ALTQ broken

2010-01-31 Thread Pyun YongHyeon
On Sun, Jan 31, 2010 at 12:37:43AM -0800, Nick Rogers wrote:
 I'm having problems getting PF + ALTQ to work on em(4) interfaces under
 8.0-RELEASE. Kernel was rebuilt with the additional options necessary for
 ALTQ and what not. Same basic configuration works fine under 7.2-RELEASE.
 Basically, the queues create successfully but running a pfctl -vsq shows a
 zero packet/byte count for all queues, even the interface's root queues.
 
 This same problem is mentioned in PR kern/138392, and the following forum
 thread...
 http://forums.freebsd.org/showthread.php?t=6656
 
 em(4)/e1000 driver from CURRENT does not fix the problem. Building an
 8.0-RELEASE kernel with the em(4) driver from 7.2-RELEASE fixes the problem
 (i.e., replacing sys/dev/e1000 with that from 7.2).
 
 One of the machines im experiencing this on has the following intel
 chipset...
 
 [u...@foo ~]$ sysctl dev.em.0
 dev.em.0.%desc: Intel(R) PRO/1000 Network Connection 6.9.6
 dev.em.0.%driver: em
 dev.em.0.%location: slot=0 function=0
 dev.em.0.%pnpinfo: vendor=0x8086 device=0x10d3 subvendor=0x15d9
 subdevice=0x040d class=0x02
 dev.em.0.%parent: pci3
 dev.em.0.debug: -1
 dev.em.0.stats: -1
 dev.em.0.rx_int_delay: 0
 dev.em.0.tx_int_delay: 66
 dev.em.0.rx_abs_int_delay: 66
 dev.em.0.tx_abs_int_delay: 66
 dev.em.0.rx_processing_limit: 100
 
 Is this issue receiving any attention? I ask because one of the em(4) driver
 contributors mentioned he had not heard of this problem in a recent thread
 regarding a different em(4) bug, and this is a pretty serious problem for me
 as I have many devices in production that need to be upgraded to 8.0, all
 running Intel interfaces and relying on ALTQ.
 
 I appreciate any updates or information and would be happy to test any
 patches and/or provide more information. Thanks.
 ___

I guess the problem comes from multi-queue support. The drbr
interface is implemented with inline function so em(4)/igb(4) may
have to define ALTQ to the header. I have not tested the patch(no
time at this moment) but would you give it try?

Thanks.
Index: sys/dev/e1000/if_igb.c
===
--- sys/dev/e1000/if_igb.c	(revision 203324)
+++ sys/dev/e1000/if_igb.c	(working copy)
@@ -36,6 +36,7 @@
 #ifdef HAVE_KERNEL_OPTION_HEADERS
 #include opt_device_polling.h
 #include opt_inet.h
+#include opt_altq.h
 #endif
 
 #include sys/param.h
Index: sys/dev/e1000/if_em.c
===
--- sys/dev/e1000/if_em.c	(revision 203324)
+++ sys/dev/e1000/if_em.c	(working copy)
@@ -35,6 +35,7 @@
 #ifdef HAVE_KERNEL_OPTION_HEADERS
 #include opt_device_polling.h
 #include opt_inet.h
+#include opt_altq.h
 #endif
 
 #include sys/param.h
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to freebsd-stable-unsubscr...@freebsd.org