[Cake] [PATCH net-next] sch_cake: Make gso-splitting configurable from userspace

[Cake] [PATCH net-next] sch_cake: Make gso-splitting configurable from userspace

[Dave Taht]

I expect the first part of this patch to generate no controversy,
as being able to enable configure gso-splitting on or off in all
use cases of cake is a goodness.

But: I expect the single line re-enabling cake's fielded default of
always splitting gro and gso packets, in shaped or unshaped mode, back
into packets, to reduce my email systems' hard disk inbox to a barren,
burnt cylinder, even if it is made easy to override thusly:

tc qdisc replace dev whatever root cake no-split-gso

While I agree that gro/gso is needed at 10gigE+ speeds, I feel
offering an option to disable splitting to those users trying to run
cake at those speeds is better than the alternative of forcing users
running at 1Gbit, 100mbit, 10mbit and below, with and without pause
frames, shaped or unshaped, to remember to split-gso.

While I have assembled tons of data in use cases ranging from nearly 0
to a gbit, the first, and most compelling argument I can make is
made in the commit that follows, where allowing GSO/GRO superpackets
triples the size of the underlying BQL when running at a gbit.

Dave Taht (1):
  sch_cake: Make gso-splitting configurable

 net/sched/sch_cake.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.7.4

[Cake] [PATCH net-next] sch_cake: Make gso-splitting configurable from userspace

[Dave Taht]

This patch restores cake's deployed behavior at line rate to always
split gso, and makes gso splitting configurable from userspace.

running cake unlimited (unshaped) at 1gigE, local traffic:

no-split-gso bql limit: 131966
split-gso bql limit:   ~42392-45420 

On this 4 stream test splitting gso apart results in halving the
observed interpacket latency at no loss in throughput.

Summary of tcp_nup test run 'gso-split' (at 2018-07-26 16:03:51.824728):

 Ping (ms) ICMP :         0.83         0.81 ms              341
 TCP upload avg :       235.43       235.39 Mbits/s         301
 TCP upload sum :       941.71       941.56 Mbits/s         301
 TCP upload::1  :       235.45       235.43 Mbits/s         271
 TCP upload::2  :       235.45       235.41 Mbits/s         289
 TCP upload::3  :       235.40       235.40 Mbits/s         288
 TCP upload::4  :       235.41       235.40 Mbits/s         291

verses

Summary of tcp_nup test run 'no-split-gso' (at 2018-07-26 16:37:23.563960):

                           avg       median          # data pts
 Ping (ms) ICMP :         1.67         1.73 ms              348
 TCP upload avg :       234.56       235.37 Mbits/s         301
 TCP upload sum :       938.24       941.49 Mbits/s         301
 TCP upload::1  :       234.55       235.38 Mbits/s         285
 TCP upload::2  :       234.57       235.37 Mbits/s         286
 TCP upload::3  :       234.58       235.37 Mbits/s         274
 TCP upload::4  :       234.54       235.42 Mbits/s         288
---
 net/sched/sch_cake.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 539c949..35fc725 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -80,7 +80,6 @@
 #define CAKE_QUEUES (1024)
 #define CAKE_FLOW_MASK 63
 #define CAKE_FLOW_NAT_FLAG 64
-#define CAKE_SPLIT_GSO_THRESHOLD (125000000) /* 1Gbps */
 
 /* struct cobalt_params - contains codel and blue parameters
  * @interval:	codel initial drop rate
@@ -2569,10 +2568,12 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
 	if (tb[TCA_CAKE_MEMORY])
 		q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
 
-	if (q->rate_bps && q->rate_bps <= CAKE_SPLIT_GSO_THRESHOLD)
-		q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
-	else
-		q->rate_flags &= ~CAKE_FLAG_SPLIT_GSO;
+	if (tb[TCA_CAKE_SPLIT_GSO]) {
+		if (!!nla_get_u32(tb[TCA_CAKE_SPLIT_GSO]))
+			q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
+		else
+			q->rate_flags &= ~CAKE_FLAG_SPLIT_GSO;
+	}
 
 	if (q->tins) {
 		sch_tree_lock(sch);
@@ -2608,7 +2609,7 @@ static int cake_init(struct Qdisc *sch, struct nlattr *opt,
 	q->target   =   5000; /* 5ms: codel RFC argues
 			       * for 5 to 10% of interval
 			       */
-
+	q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
 	q->cur_tin = 0;
 	q->cur_flow  = 0;
 
-- 
2.7.4

Re: [Cake] [PATCH net-next] sch_cake: Make gso-splitting configurable from userspace

[Dave Taht]

I should have made that be an RFC. Ah, well.
On Thu, Jul 26, 2018 at 7:45 PM Dave Taht <dave.taht@gmail.com> wrote:
>
> This patch restores cake's deployed behavior at line rate to always
> split gso, and makes gso splitting configurable from userspace.
>
> running cake unlimited (unshaped) at 1gigE, local traffic:
>
> no-split-gso bql limit: 131966
> split-gso bql limit:   ~42392-45420
>
> On this 4 stream test splitting gso apart results in halving the
> observed interpacket latency at no loss in throughput.
>
> Summary of tcp_nup test run 'gso-split' (at 2018-07-26 16:03:51.824728):
>
>  Ping (ms) ICMP :         0.83         0.81 ms              341
>  TCP upload avg :       235.43       235.39 Mbits/s         301
>  TCP upload sum :       941.71       941.56 Mbits/s         301
>  TCP upload::1  :       235.45       235.43 Mbits/s         271
>  TCP upload::2  :       235.45       235.41 Mbits/s         289
>  TCP upload::3  :       235.40       235.40 Mbits/s         288
>  TCP upload::4  :       235.41       235.40 Mbits/s         291
>
> verses
>
> Summary of tcp_nup test run 'no-split-gso' (at 2018-07-26 16:37:23.563960):
>
>                            avg       median          # data pts
>  Ping (ms) ICMP :         1.67         1.73 ms              348
>  TCP upload avg :       234.56       235.37 Mbits/s         301
>  TCP upload sum :       938.24       941.49 Mbits/s         301
>  TCP upload::1  :       234.55       235.38 Mbits/s         285
>  TCP upload::2  :       234.57       235.37 Mbits/s         286
>  TCP upload::3  :       234.58       235.37 Mbits/s         274
>  TCP upload::4  :       234.54       235.42 Mbits/s         288
> ---
>  net/sched/sch_cake.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index 539c949..35fc725 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c
> @@ -80,7 +80,6 @@
>  #define CAKE_QUEUES (1024)
>  #define CAKE_FLOW_MASK 63
>  #define CAKE_FLOW_NAT_FLAG 64
> -#define CAKE_SPLIT_GSO_THRESHOLD (125000000) /* 1Gbps */
>
>  /* struct cobalt_params - contains codel and blue parameters
>   * @interval:  codel initial drop rate
> @@ -2569,10 +2568,12 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
>         if (tb[TCA_CAKE_MEMORY])
>                 q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
>
> -       if (q->rate_bps && q->rate_bps <= CAKE_SPLIT_GSO_THRESHOLD)
> -               q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
> -       else
> -               q->rate_flags &= ~CAKE_FLAG_SPLIT_GSO;
> +       if (tb[TCA_CAKE_SPLIT_GSO]) {
> +               if (!!nla_get_u32(tb[TCA_CAKE_SPLIT_GSO]))
> +                       q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
> +               else
> +                       q->rate_flags &= ~CAKE_FLAG_SPLIT_GSO;
> +       }
>
>         if (q->tins) {
>                 sch_tree_lock(sch);
> @@ -2608,7 +2609,7 @@ static int cake_init(struct Qdisc *sch, struct nlattr *opt,
>         q->target   =   5000; /* 5ms: codel RFC argues
>                                * for 5 to 10% of interval
>                                */
> -
> +       q->rate_flags |= CAKE_FLAG_SPLIT_GSO;
>         q->cur_tin = 0;
>         q->cur_flow  = 0;
>
> --
> 2.7.4
>


-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

Re: [Cake] [PATCH net-next] sch_cake: Make gso-splitting configurable from userspace

[David Miller]

From: Dave Taht <dave.taht@gmail.com>
Date: Thu, 26 Jul 2018 19:45:09 -0700

> I expect the first part of this patch to generate no controversy,
> as being able to enable configure gso-splitting on or off in all
> use cases of cake is a goodness.
> 
> But: I expect the single line re-enabling cake's fielded default of
> always splitting gro and gso packets, in shaped or unshaped mode, back
> into packets, to reduce my email systems' hard disk inbox to a barren,
> burnt cylinder, even if it is made easy to override thusly:
> 
> tc qdisc replace dev whatever root cake no-split-gso
> 
> While I agree that gro/gso is needed at 10gigE+ speeds, I feel
> offering an option to disable splitting to those users trying to run
> cake at those speeds is better than the alternative of forcing users
> running at 1Gbit, 100mbit, 10mbit and below, with and without pause
> frames, shaped or unshaped, to remember to split-gso.
> 
> While I have assembled tons of data in use cases ranging from nearly 0
> to a gbit, the first, and most compelling argument I can make is
> made in the commit that follows, where allowing GSO/GRO superpackets
> triples the size of the underlying BQL when running at a gbit.

I've applied this, we can revert it if there are strong objections.