Re: [Cake] [PATCH 3/4] sch_api: Allow reducing queue backlog by a negative value

2019-01-07 Thread Eric Dumazet


On 01/07/2019 11:47 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen 
> 
> With GSO splitting in sch_cake, we can decrease as well as increase the
> qlen. To make it possible to signal this up the qdisc tree, change
> qdisc_tree_reduce_backlog() to accept signed integer values as the number
> of packets and bytes to reduce the backlog by.


Not sure why this patch is needed, given we use u32 integers
for q.qlen & qstats.backlog

0x for n is really -1 once folded...

sch->q.qlen -= n;
sch->qstats.backlog -= len;




___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


Re: [Cake] [PATCH 2/4] sched: Fix detection of empty queues in child qdiscs

2019-01-07 Thread Dave Taht
awesome. I'd (rarely) seen this bug in drr and qfq and never solved
it, thus going for the all-in-one fq_codel

On Mon, Jan 7, 2019 at 11:48 AM Toke Høiland-Jørgensen  wrote:
>
> From: Toke Høiland-Jørgensen 
>
> Several qdiscs check on enqueue whether the packet was enqueued to a class
> with an empty queue, in which case the class is activated. This is done by
> checking if the qlen is exactly 1 after enqueue. However, if GSO splitting
> is enabled in the child qdisc, a single packet can result in a qlen longer
> than 1. This means the activation check fails, leading to a stalled queue.
>
> Fix this by checking if the queue is empty *before* enqueue, and running
> the activation logic if this was the case.
>
> Reported-by: Pete Heist 
> Signed-off-by: Toke Høiland-Jørgensen 
> ---
>  net/sched/sch_drr.c  | 4 +++-
>  net/sched/sch_hfsc.c | 4 +++-
>  net/sched/sch_qfq.c  | 4 +++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
> index feaf47178653..09b800991065 100644
> --- a/net/sched/sch_drr.c
> +++ b/net/sched/sch_drr.c
> @@ -354,6 +354,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc 
> *sch,
> struct drr_sched *q = qdisc_priv(sch);
> struct drr_class *cl;
> int err = 0;
> +   bool first;
>
> cl = drr_classify(skb, sch, );
> if (cl == NULL) {
> @@ -363,6 +364,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc 
> *sch,
> return err;
> }
>
> +   first = !cl->qdisc->q.qlen;
> err = qdisc_enqueue(skb, cl->qdisc, to_free);
> if (unlikely(err != NET_XMIT_SUCCESS)) {
> if (net_xmit_drop_count(err)) {
> @@ -372,7 +374,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc 
> *sch,
> return err;
> }
>
> -   if (cl->qdisc->q.qlen == 1) {
> +   if (first) {
> list_add_tail(>alist, >active);
> cl->deficit = cl->quantum;
> }
> diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
> index 6bb8f73a8473..24cc220a3218 100644
> --- a/net/sched/sch_hfsc.c
> +++ b/net/sched/sch_hfsc.c
> @@ -1542,6 +1542,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
> struct sk_buff **to_free)
> unsigned int len = qdisc_pkt_len(skb);
> struct hfsc_class *cl;
> int uninitialized_var(err);
> +   bool first;
>
> cl = hfsc_classify(skb, sch, );
> if (cl == NULL) {
> @@ -1551,6 +1552,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
> struct sk_buff **to_free)
> return err;
> }
>
> +   first = !cl->qdisc->q.qlen;
> err = qdisc_enqueue(skb, cl->qdisc, to_free);
> if (unlikely(err != NET_XMIT_SUCCESS)) {
> if (net_xmit_drop_count(err)) {
> @@ -1560,7 +1562,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
> struct sk_buff **to_free)
> return err;
> }
>
> -   if (cl->qdisc->q.qlen == 1) {
> +   if (first) {
> if (cl->cl_flags & HFSC_RSC)
> init_ed(cl, len);
> if (cl->cl_flags & HFSC_FSC)
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 8d5e55d5bed2..29f5c4a24688 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -1215,6 +1215,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct 
> Qdisc *sch,
> struct qfq_class *cl;
> struct qfq_aggregate *agg;
> int err = 0;
> +   bool first;
>
> cl = qfq_classify(skb, sch, );
> if (cl == NULL) {
> @@ -1236,6 +1237,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct 
> Qdisc *sch,
> }
>
> gso_segs = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1;
> +   first = !cl->qdisc->q.qlen;
> err = qdisc_enqueue(skb, cl->qdisc, to_free);
> if (unlikely(err != NET_XMIT_SUCCESS)) {
> pr_debug("qfq_enqueue: enqueue failed %d\n", err);
> @@ -1253,7 +1255,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct 
> Qdisc *sch,
>
> agg = cl->agg;
> /* if the queue was not empty, then done here */
> -   if (cl->qdisc->q.qlen != 1) {
> +   if (!first) {
> if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
> list_first_entry(>active, struct qfq_class, alist)
> == cl && cl->deficit < len)
> --
> 2.20.1
>
> ___
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake



-- 

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


[Cake] [PATCH 4/4] sch_cake: Correctly update parent qlen when splitting GSO packets

2019-01-07 Thread Toke Høiland-Jørgensen
From: Toke Høiland-Jørgensen 

To ensure parent qdiscs have the same notion of the number of enqueued
packets even after splitting a GSO packet, update the qdisc tree with the
number of packets that was added due to the split.

Signed-off-by: Toke Høiland-Jørgensen 
---
 net/sched/sch_cake.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index b910cd5c56f7..73940293700d 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1667,7 +1667,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
if (skb_is_gso(skb) && q->rate_flags & CAKE_FLAG_SPLIT_GSO) {
struct sk_buff *segs, *nskb;
netdev_features_t features = netif_skb_features(skb);
-   unsigned int slen = 0;
+   unsigned int slen = 0, numsegs = 0;
 
segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
if (IS_ERR_OR_NULL(segs))
@@ -1683,6 +1683,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
flow_queue_add(flow, segs);
 
sch->q.qlen++;
+   numsegs++;
slen += segs->len;
q->buffer_used += segs->truesize;
b->packets++;
@@ -1696,7 +1697,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
sch->qstats.backlog += slen;
q->avg_window_bytes += slen;
 
-   qdisc_tree_reduce_backlog(sch, 1, len);
+   qdisc_tree_reduce_backlog(sch, 1-numsegs, len-slen);
consume_skb(skb);
} else {
/* not splitting */
-- 
2.20.1

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


[Cake] [PATCH 0/4] sched: Fix qdisc interactions exposed by using sch_cake as a leaf qdisc

2019-01-07 Thread Toke Høiland-Jørgensen
This series fixes a couple of issues exposed by running sch_cake as a
leaf qdisc in an HFSC tree, which were discovered and reported by Pete
Heist. The interaction between CAKE's GSO splitting and the parent
qdisc's notion of its own queue length could cause queue stalls. While
investigating the report, I also noticed that several qdiscs would
dereference the skb pointer after dequeue, which is potentially
problematic since the GSO splitting code also frees the original skb.

See the individual patches in the series for details.

Toke Høiland-Jørgensen (4):
  sched: Avoid dereferencing skb pointer after child enqueue
  sched: Fix detection of empty queues in child qdiscs
  sch_api: Allow reducing queue backlog by a negative value
  sch_cake: Correctly update parent qlen when splitting GSO packets

 include/net/sch_generic.h |  3 +--
 net/sched/sch_api.c   |  3 +--
 net/sched/sch_cake.c  |  5 +++--
 net/sched/sch_cbs.c   |  3 ++-
 net/sched/sch_drr.c   |  7 +--
 net/sched/sch_dsmark.c|  3 ++-
 net/sched/sch_hfsc.c  |  9 +
 net/sched/sch_htb.c   |  3 ++-
 net/sched/sch_prio.c  |  3 ++-
 net/sched/sch_qfq.c   | 20 
 net/sched/sch_tbf.c   |  3 ++-
 11 files changed, 37 insertions(+), 25 deletions(-)

-- 
2.20.1

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


[Cake] [PATCH 1/4] sched: Avoid dereferencing skb pointer after child enqueue

2019-01-07 Thread Toke Høiland-Jørgensen
From: Toke Høiland-Jørgensen 

Parent qdiscs may dereference the pointer to the enqueued skb after
enqueue. However, both CAKE and TBF call consume_skb() on the original skb
when splitting GSO packets, leading to a potential use-after-free in the
parent. Fix this by avoiding dereferencing the skb pointer after enqueueing
to the child.

Signed-off-by: Toke Høiland-Jørgensen 
---
 net/sched/sch_cbs.c|  3 ++-
 net/sched/sch_drr.c|  3 ++-
 net/sched/sch_dsmark.c |  3 ++-
 net/sched/sch_hfsc.c   |  5 ++---
 net/sched/sch_htb.c|  3 ++-
 net/sched/sch_prio.c   |  3 ++-
 net/sched/sch_qfq.c| 16 +---
 net/sched/sch_tbf.c|  3 ++-
 8 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index e689e11b6d0f..c6a502933fe7 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -88,13 +88,14 @@ static int cbs_child_enqueue(struct sk_buff *skb, struct 
Qdisc *sch,
 struct Qdisc *child,
 struct sk_buff **to_free)
 {
+   unsigned int len = qdisc_pkt_len(skb);
int err;
 
err = child->ops->enqueue(skb, child, to_free);
if (err != NET_XMIT_SUCCESS)
return err;
 
-   qdisc_qstats_backlog_inc(sch, skb);
+   sch->qstats.backlog += len;
sch->q.qlen++;
 
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index cdebaed0f8cf..feaf47178653 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -350,6 +350,7 @@ static struct drr_class *drr_classify(struct sk_buff *skb, 
struct Qdisc *sch,
 static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
   struct sk_buff **to_free)
 {
+   unsigned int len = qdisc_pkt_len(skb);
struct drr_sched *q = qdisc_priv(sch);
struct drr_class *cl;
int err = 0;
@@ -376,7 +377,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
cl->deficit = cl->quantum;
}
 
-   qdisc_qstats_backlog_inc(sch, skb);
+   sch->qstats.backlog += len;
sch->q.qlen++;
return err;
 }
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index f6f480784bc6..42471464ded3 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -199,6 +199,7 @@ static struct tcf_block *dsmark_tcf_block(struct Qdisc 
*sch, unsigned long cl,
 static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch,
  struct sk_buff **to_free)
 {
+   unsigned int len = qdisc_pkt_len(skb);
struct dsmark_qdisc_data *p = qdisc_priv(sch);
int err;
 
@@ -271,7 +272,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
return err;
}
 
-   qdisc_qstats_backlog_inc(sch, skb);
+   sch->qstats.backlog += len;
sch->q.qlen++;
 
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index b18ec1f6de60..6bb8f73a8473 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1539,6 +1539,7 @@ hfsc_dump_qdisc(struct Qdisc *sch, struct sk_buff *skb)
 static int
 hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 {
+   unsigned int len = qdisc_pkt_len(skb);
struct hfsc_class *cl;
int uninitialized_var(err);
 
@@ -1560,8 +1561,6 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
struct sk_buff **to_free)
}
 
if (cl->qdisc->q.qlen == 1) {
-   unsigned int len = qdisc_pkt_len(skb);
-
if (cl->cl_flags & HFSC_RSC)
init_ed(cl, len);
if (cl->cl_flags & HFSC_FSC)
@@ -1576,7 +1575,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, 
struct sk_buff **to_free)
 
}
 
-   qdisc_qstats_backlog_inc(sch, skb);
+   sch->qstats.backlog += len;
sch->q.qlen++;
 
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 58b449490757..30f9da7e1076 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -581,6 +581,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
   struct sk_buff **to_free)
 {
int uninitialized_var(ret);
+   unsigned int len = qdisc_pkt_len(skb);
struct htb_sched *q = qdisc_priv(sch);
struct htb_class *cl = htb_classify(skb, sch, );
 
@@ -610,7 +611,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
htb_activate(q, cl);
}
 
-   qdisc_qstats_backlog_inc(sch, skb);
+   sch->qstats.backlog += len;
sch->q.qlen++;
return NET_XMIT_SUCCESS;
 }
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index cdf68706e40f..847141cd900f 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -72,6 +72,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int 
*qerr)
 static int
 prio_enqueue(struct sk_buff *skb, struct Qdisc 

[Cake] [PATCH 3/4] sch_api: Allow reducing queue backlog by a negative value

2019-01-07 Thread Toke Høiland-Jørgensen
From: Toke Høiland-Jørgensen 

With GSO splitting in sch_cake, we can decrease as well as increase the
qlen. To make it possible to signal this up the qdisc tree, change
qdisc_tree_reduce_backlog() to accept signed integer values as the number
of packets and bytes to reduce the backlog by.

Signed-off-by: Toke Høiland-Jørgensen 
---
 include/net/sch_generic.h | 3 +--
 net/sched/sch_api.c   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9481f2c142e2..7a4957599874 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -580,8 +580,7 @@ struct Qdisc *dev_graft_qdisc(struct netdev_queue 
*dev_queue,
 void qdisc_reset(struct Qdisc *qdisc);
 void qdisc_put(struct Qdisc *qdisc);
 void qdisc_put_unlocked(struct Qdisc *qdisc);
-void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, unsigned int n,
-  unsigned int len);
+void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, int n, int len);
 #ifdef CONFIG_NET_SCHED
 int qdisc_offload_dump_helper(struct Qdisc *q, enum tc_setup_type type,
  void *type_data);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 7e4d1ccf4c87..03e26e8d0ec9 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -758,8 +758,7 @@ static u32 qdisc_alloc_handle(struct net_device *dev)
return 0;
 }
 
-void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n,
-  unsigned int len)
+void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
 {
bool qdisc_is_offloaded = sch->flags & TCQ_F_OFFLOADED;
const struct Qdisc_class_ops *cops;
-- 
2.20.1

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


Re: [Cake] host hashes and NAT neglected in src/dst-host mode

2019-01-07 Thread Georgios Amanakis
Yes, of course! I was counting the flow-modes wrong.
Sorry for the confusion.

On Mon, Jan 7, 2019 at 3:20 AM Jonathan Morton  wrote:
>
> > On 7 Jan, 2019, at 6:00 am,   
> > wrote:
> >
> > 633 if ((flow_override || !(flow_mode & CAKE_FLOW_FLOWS)) &&
> > 634 (host_override || !(flow_mode & CAKE_FLOW_HOSTS)))
> > 635 goto skip_hash;
>
> These lines require careful reading.
>
> First, the "override" flags indicate whether an external filter has changed 
> the flow or host hashes, meaning we should not then update them ourselves.
>
> Secondly, the logic is "if we *don't* need the flow hash *and* we *don't* 
> need the host hash, then skip the complicated hash code".
>
> In the dual and triple modes, both the flow and host hashes are required, and 
> bit-level examination of the codes used to identify them should reflect that. 
>  In "flows" and "host" modes, only one or the other are needed, but they will 
> still disable the above check (unless an external filter was used).
>
> In short, only "flowblind" mode or the use of external filters are capable of 
> skipping that block.
>
>  - Jonathan Morton
>
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


Re: [Cake] cake infinite loop(?) with hfsc on one-armed router

2019-01-07 Thread Pete Heist
Sorry, that’s without the patch, will give that a try when I have a chance and 
post the results, probably tomorrow...

> On Jan 7, 2019, at 12:30 PM, Toke Høiland-Jørgensen  wrote:
> 
> Pete Heist  writes:
> 
>>> On Jan 6, 2019, at 9:56 PM, Toke Høiland-Jørgensen  wrote:
>>> 
>>> Pete Heist  writes:
>>> 
 Lastly, is using cake as a leaf to htb risky until a fix is made? I’ve
 been doing that for a while without any apparent issues, though I’m
 hesitating now to try that in a production environment.
>>> 
>>> Hmm, that's a good question. I would expect so; but I would also expect
>>> the issue to show up pretty much straight away, so if you haven't hit it
>>> yet, I may be wrong. I'll do some more digging... Should probably also
>>> try to replicate all this stuff on my own machine :)
>> 
>> 
>> Ok, after what I’m seeing on my APU1 tests on 3.16.7, I’m definitely
>> not putting split GSO into production. I just turned it on and off
>> three times and here’s what I got:
>> 
>> Split GSO on:
>> 
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso.svg
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso2.svg
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso3.svg
>> 
>> Split GSO off:
>> 
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso.svg
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso2.svg
>> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso3.svg
>> 
>> I’ve seen these square waves before with htb and wondered where they
>> came from, and I think we may finally have an answer! What manner of
>> thing causes this I don’t know, but there’s a chance you may end up
>> finding out… :)
> 
> Is this without the patch to CAKE that adjusts the qlen? And have you
> tried running with that patch (with HTB)?
> 
> -Toke

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


Re: [Cake] cake infinite loop(?) with hfsc on one-armed router

2019-01-07 Thread Toke Høiland-Jørgensen
Pete Heist  writes:

>> On Jan 6, 2019, at 9:56 PM, Toke Høiland-Jørgensen  wrote:
>> 
>> Pete Heist  writes:
>> 
>>> Lastly, is using cake as a leaf to htb risky until a fix is made? I’ve
>>> been doing that for a while without any apparent issues, though I’m
>>> hesitating now to try that in a production environment.
>> 
>> Hmm, that's a good question. I would expect so; but I would also expect
>> the issue to show up pretty much straight away, so if you haven't hit it
>> yet, I may be wrong. I'll do some more digging... Should probably also
>> try to replicate all this stuff on my own machine :)
>
>
> Ok, after what I’m seeing on my APU1 tests on 3.16.7, I’m definitely
> not putting split GSO into production. I just turned it on and off
> three times and here’s what I got:
>
> Split GSO on:
>
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso.svg
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso2.svg
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_split_gso3.svg
>
> Split GSO off:
>
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso.svg
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso2.svg
> https://www.heistp.net/downloads/htb_split_gso/htb_cake_no_split_gso3.svg
>
> I’ve seen these square waves before with htb and wondered where they
> came from, and I think we may finally have an answer! What manner of
> thing causes this I don’t know, but there’s a chance you may end up
> finding out… :)

Is this without the patch to CAKE that adjusts the qlen? And have you
tried running with that patch (with HTB)?

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake


Re: [Cake] host hashes and NAT neglected in src/dst-host mode

2019-01-07 Thread Jonathan Morton
> On 7 Jan, 2019, at 6:00 am,   wrote:
> 
> 633 if ((flow_override || !(flow_mode & CAKE_FLOW_FLOWS)) &&
> 634 (host_override || !(flow_mode & CAKE_FLOW_HOSTS)))
> 635 goto skip_hash;

These lines require careful reading.

First, the "override" flags indicate whether an external filter has changed the 
flow or host hashes, meaning we should not then update them ourselves.

Secondly, the logic is "if we *don't* need the flow hash *and* we *don't* need 
the host hash, then skip the complicated hash code".

In the dual and triple modes, both the flow and host hashes are required, and 
bit-level examination of the codes used to identify them should reflect that.  
In "flows" and "host" modes, only one or the other are needed, but they will 
still disable the above check (unless an external filter was used).

In short, only "flowblind" mode or the use of external filters are capable of 
skipping that block.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake