Re: [PATCH v2] net: sched: tbf: handle GSO_BY_FRAGS case in enqueue

[David Miller]

From: Daniel Axtens 
Date: Mon,  5 Feb 2018 15:02:06 +1100

> tbf_enqueue() checks the size of a packet before enqueuing it.
> However, the GSO size check does not consider the GSO_BY_FRAGS
> case, and so will drop GSO SCTP packets, causing a massive drop
> in throughput.
> 
> Use skb_gso_validate_mac_len() instead, as it does consider that
> case.
> 
> Signed-off-by: Daniel Axtens 
> 
> ---
> 
> skb_gso_validate_mac_len() is an out-of-line call, but so is
> skb_gso_mac_seglen(), so this is slower but not much slower. I
> will send a patch to make the skb_gso_validate_* functions
> inline-able shortly.
> 
> Also, GSO_BY_FRAGS considered harmful - I'm pretty sure this is
> not the only place it causes issues.
> 
> v2: put S-o-b in the right spot, thanks Andrew Donnellan

It's not good that our GSO helpers are not universal, and do
not properly handle all kinds of GSO encodings the kernel can
produce.

Like you said this problem probably exists elsewhere.

Therefore, I would much rather you fix the helpers to handle
GSO_BY_FRAGS properly.

[PATCH v2] net: sched: tbf: handle GSO_BY_FRAGS case in enqueue

[Daniel Axtens]

tbf_enqueue() checks the size of a packet before enqueuing it.
However, the GSO size check does not consider the GSO_BY_FRAGS
case, and so will drop GSO SCTP packets, causing a massive drop
in throughput.

Use skb_gso_validate_mac_len() instead, as it does consider that
case.

Signed-off-by: Daniel Axtens 

---

skb_gso_validate_mac_len() is an out-of-line call, but so is
skb_gso_mac_seglen(), so this is slower but not much slower. I
will send a patch to make the skb_gso_validate_* functions
inline-able shortly.

Also, GSO_BY_FRAGS considered harmful - I'm pretty sure this is
not the only place it causes issues.

v2: put S-o-b in the right spot, thanks Andrew Donnellan

---
 net/sched/sch_tbf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 229172d509cc..03225a8df973 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -188,7 +188,8 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc 
*sch,
int ret;
 
if (qdisc_pkt_len(skb) > q->max_size) {
-   if (skb_is_gso(skb) && skb_gso_mac_seglen(skb) <= q->max_size)
+   if (skb_is_gso(skb) &&
+   skb_gso_validate_mac_len(skb, q->max_size))
return tbf_segment(skb, sch, to_free);
return qdisc_drop(skb, sch, to_free);
}
-- 
2.14.1