Requesting comments. So, bacically commit 56b765b79 (htb: improved accuracy at high rates), broke the "linklayer atm" handling. As it didn't update the iproute tc util.
Treating this as a regression fix, this is the smallest and least intrusive solution I could come up with. I'm basically restoring the "linklayer atm" handling, by using the __reserved field in struct tc_ratespec, to convey the chosen linklayer option. KERNEL patch: ============= [PATCH RFC] net_sched: restore "linklayer atm" handling From: Jesper Dangaard Brouer <[email protected]> commit 56b765b79 ("htb: improved accuracy at high rates") broke the "linklayer atm" handling. tc class add ... htb rate X ceil Y linklayer atm This patch restores the "linklayer atm" handling, by using the __reserved field in struct tc_ratespec, to convey the choosen linklayer option. This requires a corrosponding iproute2 tc fix, that updates this field. Older tc binaries can be detected by the kernel, as the field would be zero. Request-For-Comments-by: Jesper Dangaard Brouer <[email protected]> --- include/net/sch_generic.h | 8 +++++++- include/uapi/linux/pkt_sched.h | 11 ++++++++++- net/sched/sch_generic.c | 4 ++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index e7f4e21..c9916b1 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -682,13 +682,18 @@ struct psched_ratecfg { u64 rate_bps; u32 mult; u16 overhead; + u8 linklayer; u8 shift; }; static inline u64 psched_l2t_ns(const struct psched_ratecfg *r, unsigned int len) { - return ((u64)(len + r->overhead) * r->mult) >> r->shift; + u64 pkt_len = len + r->overhead; + + if (r->linklayer == TC_LINKLAYER_ATM) + pkt_len = DIV_ROUND_UP(pkt_len,48)*53; + return (pkt_len * r->mult) >> r->shift; } extern void psched_ratecfg_precompute(struct psched_ratecfg *r, const struct tc_ratespec *conf); @@ -699,6 +704,7 @@ static inline void psched_ratecfg_getrate(struct tc_ratespec *res, memset(res, 0, sizeof(*res)); res->rate = r->rate_bps >> 3; res->overhead = r->overhead; + res->linklayer = r->linklayer; } #endif diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h index dbd71b0..71e1463 100644 --- a/include/uapi/linux/pkt_sched.h +++ b/include/uapi/linux/pkt_sched.h @@ -73,9 +73,18 @@ struct tc_estimator { #define TC_H_ROOT (0xFFFFFFFFU) #define TC_H_INGRESS (0xFFFFFFF1U) +/* NOTES: Move to another .h file??? + * NOTES: Need to corrospond to iproute2 tc/tc_core.h "enum link_layer". + */ +enum tc_link_layer { + TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */ + TC_LINKLAYER_ETHERNET, + TC_LINKLAYER_ATM, +}; + struct tc_ratespec { unsigned char cell_log; - unsigned char __reserved; + __u8 linklayer; unsigned short overhead; short cell_align; unsigned short mpu; diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 2022408..8ff9a5e 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -908,6 +908,10 @@ void psched_ratecfg_precompute(struct psched_ratecfg *r, memset(r, 0, sizeof(*r)); r->overhead = conf->overhead; r->rate_bps = (u64)conf->rate << 3; + r->linklayer = conf->linklayer; + /* We could add a warn once here: + * if (r->linklayer == TC_LINKLAYER_UNAWARE) + */ r->mult = 1; /* * Calibrate mult, shift so that token counting is accurate IPROUTE2 patch ============== tc: convey linklayer information to the kernel From: Jesper Dangaard Brouer <[email protected]> --- include/linux/pkt_sched.h | 8 +++++++- tc/q_htb.c | 2 ++ tc/tc_core.c | 1 + 3 files changed, 10 insertions(+), 1 deletions(-) diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h index dbd71b0..8abdae6 100644 --- a/include/linux/pkt_sched.h +++ b/include/linux/pkt_sched.h @@ -73,9 +73,15 @@ struct tc_estimator { #define TC_H_ROOT (0xFFFFFFFFU) #define TC_H_INGRESS (0xFFFFFFF1U) +enum tc_link_layer { + TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */ + TC_LINKLAYER_ETHERNET, + TC_LINKLAYER_ATM, +}; + struct tc_ratespec { unsigned char cell_log; - unsigned char __reserved; + __u8 linklayer; unsigned short overhead; short cell_align; unsigned short mpu; diff --git a/tc/q_htb.c b/tc/q_htb.c index caa47c2..9b17412 100644 --- a/tc/q_htb.c +++ b/tc/q_htb.c @@ -267,6 +267,8 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt) buffer = tc_calc_xmitsize(hopt->rate.rate, hopt->buffer); fprintf(f, "ceil %s ", sprint_rate(hopt->ceil.rate, b1)); cbuffer = tc_calc_xmitsize(hopt->ceil.rate, hopt->cbuffer); + // TODO: ADD print statements for linklayer + if (show_details) { fprintf(f, "burst %s/%u mpu %s overhead %s ", sprint_size(buffer, b1), diff --git a/tc/tc_core.c b/tc/tc_core.c index 85b072e..9383fdc 100644 --- a/tc/tc_core.c +++ b/tc/tc_core.c @@ -129,6 +129,7 @@ int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, rtab[i] = tc_calc_xmittime(bps, sz); } + r->linklayer = linklayer; r->cell_align=-1; // Due to the sz calc r->cell_log=cell_log; return cell_log; _______________________________________________ Bloat mailing list [email protected] https://lists.bufferbloat.net/listinfo/bloat
