diff mbox

net: make CONFIG_BQL actually end user configurable

Message ID 1331005096-12332-1-git-send-email-paul.gortmaker@windriver.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Paul Gortmaker March 6, 2012, 3:38 a.m. UTC
Without the defining string or help text, LKC won't ever bother
to ask the end user for a setting for CONFIG_BQL -- you could
delete it from your .config and run make oldconfig and not a
thing would change -- it would still be silently re-enabled.

While most people will have no reason to turn this off, the
ability to do so can be useful for testing BQL support additions
on previously BQL-unaware drivers and similar.

The kconfig help text is largely taken from the original RFC
patchset 0/N header sent to netdev@vger.kernel.org in fall 2011.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---

[Apologies if this was explicitly blocked for a reason; I couldn't
 find a reason after searching netdev or threads at bufferbloat.net ]

Comments

Paul Gortmaker March 6, 2012, 4:09 a.m. UTC | #1
[Re: [PATCH] net: make CONFIG_BQL actually end user configurable] On 05/03/2012 (Mon 19:45) Dave Taht wrote:

> 
> 
> On Mon, Mar 5, 2012 at 7:38 PM, Paul Gortmaker <paul.gortmaker@windriver.com>
> wrote:
> 
>     Without the defining string or help text, LKC won't ever bother
>     to ask the end user for a setting for CONFIG_BQL -- you could
>     delete it from your .config and run make oldconfig and not a
>     thing would change -- it would still be silently re-enabled.
> 
> 
> 
> My specific problems with BQL as currently implemented are:
> 
> 1) There is no way to tell if a driver has it actually enabled or not
> at run time.

In a similar vein, I was interested in a (quick) way to tell if a
driver has it actually enabled *correctly*.  I'd coded up the basic
changes for a common PPC NIC, but I was then left wondering what I
could use as a quick high-water-mark to see if I'd really managed
to implement it correctly via some level of real run-time testing.

If we want to get driver authors involved in adding support to their
favourite hardware, some description of a basic sanity test would
probably be a worthwhile description to have.

Paul.
--

> 
> 2) Down below 100Mbit (the range that I care about) the controller
> can automatically allocate more ram than I would like for the outstanding
> data. I can (and do) easily control this by overriding the BQL default
> to something that makes things behave closer to what a ns2 model
> would predict (about 3k seems optimal) without affecting throughput.
> 
> whether or not this is a problem in the real world is yet to be seen.
>  
> 
>     While most people will have no reason to turn this off, the
>     ability to do so can be useful for testing BQL support additions
>     on previously BQL-unaware drivers and similar.
> 
>     The kconfig help text is largely taken from the original RFC
>     patchset 0/N header sent to netdev@vger.kernel.org in fall 2011.
> 
>     Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>     ---
> 
>     [Apologies if this was explicitly blocked for a reason; I couldn't
>      find a reason after searching netdev or threads at bufferbloat.net ]
> 
>     diff --git a/net/Kconfig b/net/Kconfig
>     index e07272d..fd1d815 100644
>     --- a/net/Kconfig
>     +++ b/net/Kconfig
>     @@ -241,10 +241,15 @@ config NETPRIO_CGROUP
>              a per-interface basis
> 
>      config BQL
>     -       boolean
>     +       boolean "Byte Queue Limits"
>            depends on SYSFS
>            select DQL
>            default y
>     +       ---help---
>     +         Byte queue limits are a mechanism to limit the size of the
>     transmit
>     +         hardware queue on a NIC by a number of bytes. The goal of these
>     byte
>     +         queue limits is to reduce latency caused by excessive queuing in
>     +         hardware without sacrificing throughput.
> 
>      config HAVE_BPF_JIT
>            bool
>     --
>     1.7.9.1
> 
>     --
>     To unsubscribe from this list: send the line "unsubscribe netdev" in
>     the body of a message to majordomo@vger.kernel.org
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> 
> --
> Dave Täht
> SKYPE: davetaht
> US Tel: 1-239-829-5608
> http://www.bufferbloat.net
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller March 6, 2012, 4:11 a.m. UTC | #2
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Mon,  5 Mar 2012 22:38:16 -0500

> While most people will have no reason to turn this off, the
> ability to do so can be useful for testing BQL support additions
> on previously BQL-unaware drivers and similar.

It's intentionally like this.

> The kconfig help text is largely taken from the original RFC
> patchset 0/N header sent to netdev@vger.kernel.org in fall 2011.

And this it why the final version that got checked in didn't
have the help text nor the user configurability.  I asked for
it to be this way.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Fastabend March 6, 2012, 4:11 a.m. UTC | #3
On 3/5/2012 7:38 PM, Paul Gortmaker wrote:
> Without the defining string or help text, LKC won't ever bother
> to ask the end user for a setting for CONFIG_BQL -- you could
> delete it from your .config and run make oldconfig and not a
> thing would change -- it would still be silently re-enabled.
> 
> While most people will have no reason to turn this off, the
> ability to do so can be useful for testing BQL support additions
> on previously BQL-unaware drivers and similar.
> 
> The kconfig help text is largely taken from the original RFC
> patchset 0/N header sent to netdev@vger.kernel.org in fall 2011.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> 
> [Apologies if this was explicitly blocked for a reason; I couldn't
>  find a reason after searching netdev or threads at bufferbloat.net ]
> 

It was intentional "The Kconfig entry and option is merely for
expressing internal dependencies,". See the comments in the patch

http://patchwork.ozlabs.org/patch/128727/

And corresponding mail,

http://lists.openwall.net/netdev/2011/12/01/130

.John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Gortmaker March 6, 2012, 4:17 a.m. UTC | #4
On Mon, Mar 5, 2012 at 11:11 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> On 3/5/2012 7:38 PM, Paul Gortmaker wrote:

[...]

>>
>> [Apologies if this was explicitly blocked for a reason; I couldn't
>>  find a reason after searching netdev or threads at bufferbloat.net ]
>>
>
> It was intentional "The Kconfig entry and option is merely for
> expressing internal dependencies,". See the comments in the patch

Thanks Dave and John.  Something told me this wasn't by accident, but
somehow I still managed to overlook this in my search.  :(

Paul.
--

>
> http://patchwork.ozlabs.org/patch/128727/
>
> And corresponding mail,
>
> http://lists.openwall.net/netdev/2011/12/01/130
>
> .John
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tom Herbert March 6, 2012, 5:57 a.m. UTC | #5
>> 1) There is no way to tell if a driver has it actually enabled or not
>> at run time.
>
> In a similar vein, I was interested in a (quick) way to tell if a
> driver has it actually enabled *correctly*.  I'd coded up the basic
> changes for a common PPC NIC, but I was then left wondering what I
> could use as a quick high-water-mark to see if I'd really managed
> to implement it correctly via some level of real run-time testing.
>
The sysfs variables should be good to view the operation and effects
of BQL.  To disable BQL (at least limit checking) "echo max >
limit_min" should be sufficient.

It is true there is no way to determine that BQL is implement for a
device, the sysfs variables are always created.  If it is
unimplemented then inflight and limit variables are always zero.

> If we want to get driver authors involved in adding support to their
> favourite hardware, some description of a basic sanity test would
> probably be a worthwhile description to have.

That would be good.  The best window into BQL is the sysfs variables
(inflight and limit).  Setting zero or max to limit_min should impact
the number of queued bytes being reported.  ("watch cat *" in tx-0I'm
still intending to get the document on BQL out, the sanity check would
be good to describe there I think.

Thanks,
Tom
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/Kconfig b/net/Kconfig
index e07272d..fd1d815 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -241,10 +241,15 @@  config NETPRIO_CGROUP
 	  a per-interface basis
 
 config BQL
-	boolean
+	boolean	"Byte Queue Limits"
 	depends on SYSFS
 	select DQL
 	default y
+	---help---
+	  Byte queue limits are a mechanism to limit the size of the transmit
+	  hardware queue on a NIC by a number of bytes. The goal of these byte
+	  queue limits is to reduce latency caused by excessive queuing in
+	  hardware without sacrificing throughput.
 
 config HAVE_BPF_JIT
 	bool