Commit 4d0820cf authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

sch_tbf: handle too small burst

If a too small burst is inadvertently set on TBF, we might trigger
a bug in tbf_segment(), as 'skb' instead of 'segs' was used in a
qdisc_reshape_fail() call.

tc qdisc add dev eth0 root handle 1: tbf latency 50ms burst 1KB rate
50mbit

Fix the bug, and add a warning, as such configuration is not
going to work anyway for non GSO packets.

(For some reason, one has to use a burst >= 1520 to get a working
configuration, even with old kernels. This is a probable iproute2/tc
bug)

Based on a report and initial patch from Yang Yingliang

Fixes: e43ac79a ("sch_tbf: segment too big GSO packets")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarYang Yingliang <yangyingliang@huawei.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 1fa4c710
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include <net/netlink.h> #include <net/netlink.h>
#include <net/sch_generic.h> #include <net/sch_generic.h>
#include <net/pkt_sched.h> #include <net/pkt_sched.h>
#include <net/tcp.h>
/* Simple Token Bucket Filter. /* Simple Token Bucket Filter.
...@@ -117,6 +118,22 @@ struct tbf_sched_data { ...@@ -117,6 +118,22 @@ struct tbf_sched_data {
}; };
/*
* Return length of individual segments of a gso packet,
* including all headers (MAC, IP, TCP/UDP)
*/
static unsigned int skb_gso_seglen(const struct sk_buff *skb)
{
unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
const struct skb_shared_info *shinfo = skb_shinfo(skb);
if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
hdr_len += tcp_hdrlen(skb);
else
hdr_len += sizeof(struct udphdr);
return hdr_len + shinfo->gso_size;
}
/* GSO packet is too big, segment it so that tbf can transmit /* GSO packet is too big, segment it so that tbf can transmit
* each segment in time * each segment in time
*/ */
...@@ -136,12 +153,8 @@ static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch) ...@@ -136,12 +153,8 @@ static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch)
while (segs) { while (segs) {
nskb = segs->next; nskb = segs->next;
segs->next = NULL; segs->next = NULL;
if (likely(segs->len <= q->max_size)) { qdisc_skb_cb(segs)->pkt_len = segs->len;
qdisc_skb_cb(segs)->pkt_len = segs->len; ret = qdisc_enqueue(segs, q->qdisc);
ret = qdisc_enqueue(segs, q->qdisc);
} else {
ret = qdisc_reshape_fail(skb, sch);
}
if (ret != NET_XMIT_SUCCESS) { if (ret != NET_XMIT_SUCCESS) {
if (net_xmit_drop_count(ret)) if (net_xmit_drop_count(ret))
sch->qstats.drops++; sch->qstats.drops++;
...@@ -163,7 +176,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch) ...@@ -163,7 +176,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
int ret; int ret;
if (qdisc_pkt_len(skb) > q->max_size) { if (qdisc_pkt_len(skb) > q->max_size) {
if (skb_is_gso(skb)) if (skb_is_gso(skb) && skb_gso_seglen(skb) <= q->max_size)
return tbf_segment(skb, sch); return tbf_segment(skb, sch);
return qdisc_reshape_fail(skb, sch); return qdisc_reshape_fail(skb, sch);
} }
...@@ -319,6 +332,11 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt) ...@@ -319,6 +332,11 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
if (max_size < 0) if (max_size < 0)
goto done; goto done;
if (max_size < psched_mtu(qdisc_dev(sch)))
pr_warn_ratelimited("sch_tbf: burst %u is lower than device %s mtu (%u) !\n",
max_size, qdisc_dev(sch)->name,
psched_mtu(qdisc_dev(sch)));
if (q->qdisc != &noop_qdisc) { if (q->qdisc != &noop_qdisc) {
err = fifo_set_limit(q->qdisc, qopt->limit); err = fifo_set_limit(q->qdisc, qopt->limit);
if (err) if (err)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment