From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 8254E3BA8E for ; Mon, 7 May 2018 12:35:51 -0400 (EDT) Received: by mail-io0-x236.google.com with SMTP id e78-v6so34841920iod.0 for ; Mon, 07 May 2018 09:35:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Weo7ZtQkABZk/h54okZhtZqGKQjJ0SKY5zSYTfNnaSQ=; b=B0RjHfsf6deIlvjo93echLXdTyxTBppyDp2edis4sii0kXyxYu2pcZ9EimV3Dvm1j2 uQMIeT8QtMtVTgzUpnw8eBqHVNMH2Krnuqi1vKu6mvmVEY910lIDkTHZHY9HzZgmVC0q O4FPkUVpr4EeorYLo0h1CQZiplUlwL4B1KikPaGTZRV4MjFP1eVNDzA5JrNMGps3voZW mz9WiWHCqJwVEvG23DJ0PTdHy/b3NT7BITyZdgIg+ivM8Yf7t87KMr1htXcOrb1RpZGq xIHHo3Uv+yk+UCF/uaDmTHlPcYSt5NXpfU76YAQW/1rV8Vps5PO5g6gYECKzMhtOioit UtQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Weo7ZtQkABZk/h54okZhtZqGKQjJ0SKY5zSYTfNnaSQ=; b=QcMd/BCuEQishlLi7TuZr5hgMz0IXVtOY3sdJrR2LtL8QyPEbZEnKIWCBTh0Tt/eTr T6G4svCsVYjg/cREgLJCmfdP8PkccJKfJ2Cz2Of+PsVkomrfclNMxFz30QDgRN29jmTn P8Wdc428Nt+DBfLlCIHzuzyQtFyuLurTrnAA+6pDyr4ZFQkwMetiiwzAqT+2T3OEiv68 hzEjFISbuvuQNGrY7MGWnwevWXIGOeYVc2E+fe7ELbOR2IwLSJ50APqplB/KE3VmdVRV aECoj9nZJivlizGY8UJHh98VmJcETkJlET60rf5gtYkFkf38LeoveS8hhHK/2qwkBsIt 8C3w== X-Gm-Message-State: ALQs6tDx6cPFqyPJkB7U5ZbXDdYbwdtswrWVXx2aYaelTDq+TirsxNH2 NNF0fyZ5kI6Mn586GDtbHRZxiZGd7omuTziH5Iw= X-Google-Smtp-Source: AB8JxZoDcE0lNUxawToHhFWjWrP2GLN90gQ/Fy6jW+3VjYj4F8lUsf2mEB0FklPdl6rMccki3HjzePmJLnvIfggsF3I= X-Received: by 2002:a63:4004:: with SMTP id n4-v6mr20363083pga.104.1525710950876; Mon, 07 May 2018 09:35:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.179.152 with HTTP; Mon, 7 May 2018 09:35:30 -0700 (PDT) In-Reply-To: <87in831bl2.fsf@toke.dk> References: <152544254217.11750.5727163821563013360.stgit@alrua-kau> <152544255298.11750.16512595539680113590.stgit@alrua-kau> <87in831bl2.fsf@toke.dk> From: Cong Wang Date: Mon, 7 May 2018 09:35:30 -0700 Message-ID: To: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= Cc: Linux Kernel Network Developers , Cake List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Mon, 07 May 2018 14:07:24 -0400 Subject: Re: [Cake] [PATCH net-next v8 1/7] sched: Add Common Applications Kept Enhanced (cake) qdisc X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 May 2018 16:35:51 -0000 On Fri, May 4, 2018 at 12:10 PM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > Thank you for the review! A few comments below, I'll fix the rest. > >> [...] >> >> So sch_cake doesn't accept normal tc filters? Is this intentional? >> If so, why? > > For two reasons: > > - The two-level scheduling used in CAKE (tins / diffserv classes, and > flow hashing) does not map in an obvious way to the classification > index of tc filters. Sounds like you need to extend struct tcf_result? > > - No one has asked for it. We have done our best to accommodate the > features people want in a home router qdisc directly in CAKE, and the > ability to integrate tc filters has never been requested. It is not hard to integrate, basically you need to call tcf_classify(). Although it is not mandatory, it is odd to merge a qdisc doesn't work with existing tc filters (and actions too). >>> +static int cake_init(struct Qdisc *sch, struct nlattr *opt, >>> + struct netlink_ext_ack *extack) >>> +{ >>> + struct cake_sched_data *q =3D qdisc_priv(sch); >>> + int i, j; >>> + >>> + sch->limit =3D 10240; >>> + q->tin_mode =3D CAKE_DIFFSERV_BESTEFFORT; >>> + q->flow_mode =3D CAKE_FLOW_TRIPLE; >>> + >>> + q->rate_bps =3D 0; /* unlimited by default */ >>> + >>> + q->interval =3D 100000; /* 100ms default */ >>> + q->target =3D 5000; /* 5ms: codel RFC argues >>> + * for 5 to 10% of interval >>> + */ >>> + >>> + q->cur_tin =3D 0; >>> + q->cur_flow =3D 0; >>> + >>> + if (opt) { >>> + int err =3D cake_change(sch, opt, extack); >>> + >>> + if (err) >>> + return err; >> >> >> Not sure if you really want to reallocate q->tines below for this >> case. > > I'm not sure what you mean here? If there's an error we return it and > the qdisc is not created. If there's not, we allocate and on subsequent > changes cake_change() will be called directly, or? Can the init function > ever be called again during the lifetime of the qdisc? > In non-error case, you call cake_change() first and then allocate ->tins with kvzalloc() below. For me it looks like you don't need to allocate it again when ->tins!=3DNULL.