[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Tue, 2 Feb 2021 13:46:29 +0200
From: Maxim Mikityanskiy <maximmi@...dia.com>
To: David Ahern <dsahern@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Jiri Pirko <jiri@...nulli.us>
CC: Saeed Mahameed <saeedm@...dia.com>,
Jakub Kicinski <kuba@...nel.org>,
Tariq Toukan <tariqt@...dia.com>,
Yossi Kuperman <yossiku@...dia.com>,
"Dan Carpenter" <dan.carpenter@...cle.com>,
<netdev@...r.kernel.org>,
"Maxim Mikityanskiy" <maximmi@...lanox.com>
Subject: Re: [PATCH iproute2-next] tc/htb: Hierarchical QoS hardware offload
On 2021-01-29 18:05, David Ahern wrote:
> On 12/15/20 12:42 AM, Maxim Mikityanskiy wrote:
>> This commit adds support for configuring HTB in offload mode. HTB
>> offload eliminates the single qdisc lock in the datapath and offloads
>> the algorithm to the NIC. The new 'offload' parameter is added to
>> enable this mode:
>>
>> # tc qdisc replace dev eth0 root handle 1: htb offload
>>
>> Classes are created as usual, but filters should be moved to clsact for
>> lock-free classification (filters attached to HTB itself are not
>> supported in the offload mode):
>>
>> # tc filter add dev eth0 egress protocol ip flower dst_port 80
>> action skbedit priority 1:10
>
> please add the dump in both stdout and json here.
Do you mean to add example output to the commit message?
>>
>> Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>
>> ---
>> include/uapi/linux/pkt_sched.h | 1 +
>> tc/q_htb.c | 9 ++++++++-
>
> missing an update to man/man8/tc-htb.8
OK, I'll add.
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
>> index 9e7c2c60..79a699f1 100644
>> --- a/include/uapi/linux/pkt_sched.h
>> +++ b/include/uapi/linux/pkt_sched.h
>> @@ -434,6 +434,7 @@ enum {
>> TCA_HTB_RATE64,
>> TCA_HTB_CEIL64,
>> TCA_HTB_PAD,
>> + TCA_HTB_OFFLOAD,
>> __TCA_HTB_MAX,
>> };
>>
>> diff --git a/tc/q_htb.c b/tc/q_htb.c
>> index c609e974..fd11dad6 100644
>> --- a/tc/q_htb.c
>> +++ b/tc/q_htb.c
>> @@ -30,11 +30,12 @@
>> static void explain(void)
>> {
>> fprintf(stderr, "Usage: ... qdisc add ... htb [default N] [r2q N]\n"
>> - " [direct_qlen P]\n"
>> + " [direct_qlen P] [offload]\n"
>> " default minor id of class to which unclassified packets are sent {0}\n"
>> " r2q DRR quantums are computed as rate in Bps/r2q {10}\n"
>> " debug string of 16 numbers each 0-3 {0}\n\n"
>> " direct_qlen Limit of the direct queue {in packets}\n"
>> + " offload hardware offload\n"
>
> why 'offload hardware offload'? does not make sense to me and
"offload" is a new parameter, and "hardware offload" is the description,
just like the other parameters above.
> you don't
> reference hardware below.
Where should I reference it?
>
>
>
>> "... class add ... htb rate R1 [burst B1] [mpu B] [overhead O]\n"
>> " [prio P] [slot S] [pslot PS]\n"
>> " [ceil R2] [cburst B2] [mtu MTU] [quantum Q]\n"
>> @@ -68,6 +69,7 @@ static int htb_parse_opt(struct qdisc_util *qu, int argc,
>> };
>> struct rtattr *tail;
>> unsigned int i; char *p;
>> + bool offload = false;
>>
>> while (argc > 0) {
>> if (matches(*argv, "r2q") == 0) {
>> @@ -91,6 +93,8 @@ static int htb_parse_opt(struct qdisc_util *qu, int argc,
>> if (get_u32(&direct_qlen, *argv, 10)) {
>> explain1("direct_qlen"); return -1;
>> }
>> + } else if (matches(*argv, "offload") == 0) {
>> + offload = true;
>> } else {
>> fprintf(stderr, "What is \"%s\"?\n", *argv);
>> explain();
>> @@ -103,6 +107,8 @@ static int htb_parse_opt(struct qdisc_util *qu, int argc,
>> if (direct_qlen != ~0U)
>> addattr_l(n, 2024, TCA_HTB_DIRECT_QLEN,
>> &direct_qlen, sizeof(direct_qlen));
>> + if (offload)
>> + addattr(n, 2024, TCA_HTB_OFFLOAD);
>> addattr_nest_end(n, tail);
>> return 0;
>> }
>> @@ -344,6 +350,7 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>> print_uint(PRINT_ANY, "direct_qlen", " direct_qlen %u",
>> direct_qlen);
>> }
>> + print_uint(PRINT_ANY, "offload", " offload %d", !!tb[TCA_HTB_OFFLOAD]);
>> return 0;
>> }
>>
>>
>
Powered by blists - more mailing lists