<div dir="ltr">jesper just dropped this set of patches on the netdev list, which I hope address some of the remaining issues on htb. There was some debate the last time this went by on the list, so the word is not in yet.<br>
<br>I note I would really like to get around to profiling the "final" aqm code in the hope of squeezing more performance out of it. Presently it tops out at about 25/25Mbit. (it's not fq_codel but htb)<br><div>
<br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Jesper Dangaard Brouer</b> <span dir="ltr"><<a href="mailto:brouer@redhat.com">brouer@redhat.com</a>></span><br>
Date: Wed, Aug 14, 2013 at 2:47 PM<br>Subject: [PATCH] net_sched: restore "linklayer atm" handling<br>To: "David S. Miller" <<a href="mailto:davem@davemloft.net">davem@davemloft.net</a>>, Dave Taht <<a href="mailto:dave.taht@gmail.com">dave.taht@gmail.com</a>>, <a href="mailto:netdev@vger.kernel.org">netdev@vger.kernel.org</a><br>
Cc: Jesper Dangaard Brouer <<a href="mailto:brouer@redhat.com">brouer@redhat.com</a>>, Stephen Hemminger <<a href="mailto:shemminger@vyatta.com">shemminger@vyatta.com</a>><br><br><br>commit 56b765b79 ("htb: improved accuracy at high rates")<br>
broke the "linklayer atm" handling.<br>
<br>
tc class add ... htb rate X ceil Y linklayer atm<br>
<br>
The linklayer setting is implemented by modifying the rate table<br>
which is send to the kernel. No direct parameter were<br>
transferred to the kernel indicating the linklayer setting.<br>
<br>
The commit 56b765b79 ("htb: improved accuracy at high rates")<br>
removed the use of the rate table system.<br>
<br>
To keep compatible with older iproute2 utils, this patch detects<br>
the linklayer by parsing the rate table. It also supports future<br>
versions of iproute2 to send this linklayer parameter to the<br>
kernel directly. This is done by using the __reserved field in<br>
struct tc_ratespec, to convey the choosen linklayer option, but<br>
only using the lower 4 bits of this field.<br>
<br>
Linklayer detection is limited to speeds below 100Mbit/s, because<br>
at high rates the rtab is gets too inaccurate, so bad that<br>
several fields contain the same values, this resembling the ATM<br>
detect. Fields even start to contain "0" time to send, e.g. at<br>
1000Mbit/s sending a 96 bytes packet cost "0", thus the rtab have<br>
been more broken than we first realized.<br>
<br>
Signed-off-by: Jesper Dangaard Brouer <<a href="mailto:brouer@redhat.com">brouer@redhat.com</a>><br>
---<br>
<br>
include/net/sch_generic.h | 9 ++++++++-<br>
include/uapi/linux/pkt_sched.h | 10 +++++++++-<br>
net/sched/sch_api.c | 41 ++++++++++++++++++++++++++++++++++++++++<br>
net/sched/sch_generic.c | 1 +<br>
net/sched/sch_htb.c | 13 +++++++++++++<br>
5 files changed, 72 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h<br>
index 6eab633..e5ae0c5 100644<br>
--- a/include/net/sch_generic.h<br>
+++ b/include/net/sch_generic.h<br>
@@ -683,13 +683,19 @@ struct psched_ratecfg {<br>
u64 rate_bytes_ps; /* bytes per second */<br>
u32 mult;<br>
u16 overhead;<br>
+ u8 linklayer;<br>
u8 shift;<br>
};<br>
<br>
static inline u64 psched_l2t_ns(const struct psched_ratecfg *r,<br>
unsigned int len)<br>
{<br>
- return ((u64)(len + r->overhead) * r->mult) >> r->shift;<br>
+ len += r->overhead;<br>
+<br>
+ if (unlikely(r->linklayer == TC_LINKLAYER_ATM))<br>
+ return ((u64)(DIV_ROUND_UP(len,48)*53) * r->mult) >> r->shift;<br>
+<br>
+ return ((u64)len * r->mult) >> r->shift;<br>
}<br>
<br>
extern void psched_ratecfg_precompute(struct psched_ratecfg *r, const struct tc_ratespec *conf);<br>
@@ -700,6 +706,7 @@ static inline void psched_ratecfg_getrate(struct tc_ratespec *res,<br>
memset(res, 0, sizeof(*res));<br>
res->rate = r->rate_bytes_ps;<br>
res->overhead = r->overhead;<br>
+ res->linklayer = (r->linklayer & TC_LINKLAYER_MASK);<br>
}<br>
<br>
#endif<br>
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h<br>
index dbd71b0..09d62b9 100644<br>
--- a/include/uapi/linux/pkt_sched.h<br>
+++ b/include/uapi/linux/pkt_sched.h<br>
@@ -73,9 +73,17 @@ struct tc_estimator {<br>
#define TC_H_ROOT (0xFFFFFFFFU)<br>
#define TC_H_INGRESS (0xFFFFFFF1U)<br>
<br>
+/* Need to corrospond to iproute2 tc/tc_core.h "enum link_layer" */<br>
+enum tc_link_layer {<br>
+ TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */<br>
+ TC_LINKLAYER_ETHERNET,<br>
+ TC_LINKLAYER_ATM,<br>
+};<br>
+#define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */<br>
+<br>
struct tc_ratespec {<br>
unsigned char cell_log;<br>
- unsigned char __reserved;<br>
+ __u8 linklayer; /* lower 4 bits */<br>
unsigned short overhead;<br>
short cell_align;<br>
unsigned short mpu;<br>
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c<br>
index 281c1bd..51b968d 100644<br>
--- a/net/sched/sch_api.c<br>
+++ b/net/sched/sch_api.c<br>
@@ -285,6 +285,45 @@ static struct Qdisc_ops *qdisc_lookup_ops(struct nlattr *kind)<br>
return q;<br>
}<br>
<br>
+/* The linklayer setting were not transferred from iproute2, in older<br>
+ * versions, and the rate tables lookup systems have been dropped in<br>
+ * the kernel. To keep backward compatible with older iproute2 tc<br>
+ * utils, we detect the linklayer setting by detecting if the rate<br>
+ * table were modified.<br>
+ *<br>
+ * For linklayer ATM table entries, the rate table will be aligned to<br>
+ * 48 bytes, thus some table entries will contain the same value. The<br>
+ * mpu (min packet unit) is also encoded into the old rate table, thus<br>
+ * starting from the mpu, we find low and high table entries for<br>
+ * mapping this cell. If these entries contain the same value, when<br>
+ * the rate tables have been modified for linklayer ATM.<br>
+ *<br>
+ * This is done by rounding mpu to the nearest 48 bytes cell/entry,<br>
+ * and then roundup to the next cell, calc the table entry one below,<br>
+ * and compare.<br>
+ */<br>
+static __u8 __detect_linklayer(struct tc_ratespec *r, __u32 *rtab)<br>
+{<br>
+ int low = roundup(r->mpu, 48);<br>
+ int high = roundup(low+1, 48);<br>
+ int cell_low = low >> r->cell_log;<br>
+ int cell_high = (high >> r->cell_log) - 1;<br>
+<br>
+ /* rtab is too inaccurate at rates > 100Mbit/s */<br>
+ if ((r->rate > (100000000/8)) || (rtab[0] == 0)) {<br>
+ pr_debug("TC linklayer: Giving up ATM detection\n");<br>
+ return TC_LINKLAYER_ETHERNET;<br>
+ }<br>
+<br>
+ if ((cell_high > cell_low) && (cell_high < 256)<br>
+ && (rtab[cell_low] == rtab[cell_high])) {<br>
+ pr_debug("TC linklayer: Detected ATM, low(%d)=high(%d)=%u\n",<br>
+ cell_low, cell_high, rtab[cell_high]);<br>
+ return TC_LINKLAYER_ATM;<br>
+ }<br>
+ return TC_LINKLAYER_ETHERNET;<br>
+}<br>
+<br>
static struct qdisc_rate_table *qdisc_rtab_list;<br>
<br>
struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct nlattr *tab)<br>
@@ -308,6 +347,8 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct nlattr *ta<br>
rtab->rate = *r;<br>
rtab->refcnt = 1;<br>
memcpy(rtab->data, nla_data(tab), 1024);<br>
+ if (r->linklayer == TC_LINKLAYER_UNAWARE)<br>
+ r->linklayer = __detect_linklayer(r, rtab->data);<br>
rtab->next = qdisc_rtab_list;<br>
qdisc_rtab_list = rtab;<br>
}<br>
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c<br>
index eeb8276..48be3d5 100644<br>
--- a/net/sched/sch_generic.c<br>
+++ b/net/sched/sch_generic.c<br>
@@ -909,6 +909,7 @@ void psched_ratecfg_precompute(struct psched_ratecfg *r,<br>
memset(r, 0, sizeof(*r));<br>
r->overhead = conf->overhead;<br>
r->rate_bytes_ps = conf->rate;<br>
+ r->linklayer = (conf->linklayer & TC_LINKLAYER_MASK);<br>
r->mult = 1;<br>
/*<br>
* The deal here is to replace a divide by a reciprocal one<br>
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c<br>
index 45e7515..c2178b1 100644<br>
--- a/net/sched/sch_htb.c<br>
+++ b/net/sched/sch_htb.c<br>
@@ -1329,6 +1329,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,<br>
struct htb_sched *q = qdisc_priv(sch);<br>
struct htb_class *cl = (struct htb_class *)*arg, *parent;<br>
struct nlattr *opt = tca[TCA_OPTIONS];<br>
+ struct qdisc_rate_table *rtab = NULL, *ctab = NULL;<br>
struct nlattr *tb[TCA_HTB_MAX + 1];<br>
struct tc_htb_opt *hopt;<br>
<br>
@@ -1350,6 +1351,18 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,<br>
if (!hopt->rate.rate || !hopt->ceil.rate)<br>
goto failure;<br>
<br>
+ /* Keeping backward compatible with rate_table based iproute2 tc */<br>
+ if (hopt->rate.linklayer == TC_LINKLAYER_UNAWARE) {<br>
+ rtab = qdisc_get_rtab(&hopt->rate, tb[TCA_HTB_RTAB]);<br>
+ if (rtab)<br>
+ qdisc_put_rtab(rtab);<br>
+ }<br>
+ if (hopt->ceil.linklayer == TC_LINKLAYER_UNAWARE) {<br>
+ ctab = qdisc_get_rtab(&hopt->ceil, tb[TCA_HTB_CTAB]);<br>
+ if (ctab)<br>
+ qdisc_put_rtab(ctab);<br>
+ }<br>
+<br>
if (!cl) { /* new class */<br>
struct Qdisc *new_q;<br>
int prio;<br>
<br>
</div><br><br clear="all"><br>-- <br>Dave Täht<br><br>Fixing bufferbloat with cerowrt: <a href="http://www.teklibre.com/cerowrt/subscribe.html" target="_blank">http://www.teklibre.com/cerowrt/subscribe.html</a>
</div></div>