| Message ID | 20180205040206.5810-1-dja@axtens.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] net: sched: tbf: handle GSO_BY_FRAGS case in enqueue | expand |
From: Daniel Axtens <dja@axtens.net> 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 <dja@axtens.net> > > --- > > 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.
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); }
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 <dja@axtens.net> --- 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(-)