From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f65.google.com (mail-ed1-f65.google.com [209.85.208.65]) (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 5FB7E3B29E for ; Mon, 4 Mar 2019 11:39:50 -0500 (EST) Received: by mail-ed1-f65.google.com with SMTP id f2so4764427edy.13 for ; Mon, 04 Mar 2019 08:39:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=nwyLXPIOTMNtB/Jw5GRQxfjnN9q/rf8n4DeeverYmPA=; b=OFLcOEy14oGSsH0X02Pg+BhUPra30COXENzUZUpdTyU/pEjTJt0RgNBkbbPgXQiqQu R89M0k5aBbVndeCco3y8amH91nYbwZwXP5GirZIN1QXKg8xDk3J9NkrQuUgvxbZo6JuK T/hG6ciZBI5y8EGWrgzJF2boPN79Vywv4sk+7JNP1fdv1IgTigcJk3KTa1sOLHZhCLB2 zWec9ezmYbaDIY2JXTM3SwZFqtioig5Sxh0KIHuLZXjOxW9OLggHu7C2RBKwvjPf4L0z 3oGP/N67gw8CC8Ogj8v3xZecHwj3eJODs0l6UCYMBavXVNEoCtg8eOWYJc7FjY57IBTp UrSg== X-Gm-Message-State: APjAAAWipbL51hJhYJUY6S2mbNdNcCe2ayS0BwqGvDhxtPM8yvNo4zZd VyIfBUfx0YqEHK532cGB7ov/Qw== X-Google-Smtp-Source: APXvYqwTmu2zCr1s1LA1j6UurcWuCtDwNM9YYPwJC0TlZ3Qk2Y35iPCeiK3ylkt1P8oJHSd/kUsoMQ== X-Received: by 2002:a50:9063:: with SMTP id z32mr16200060edz.164.1551717589457; Mon, 04 Mar 2019 08:39:49 -0800 (PST) Received: from alrua-x1.borgediget.toke.dk (alrua-x1.vpn.toke.dk. [2a00:7660:6da:10::2]) by smtp.gmail.com with ESMTPSA id cd11sm1274303ejb.61.2019.03.04.08.39.48 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 04 Mar 2019 08:39:48 -0800 (PST) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 748F3182E8B; Mon, 4 Mar 2019 17:39:47 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Kevin Darbyshire-Bryant Cc: Pete Heist , "cake\@lists.bufferbloat.net" In-Reply-To: References: <1443315187.406067.1551301968084@webmail.strato.de> <000901d4cf15$27748ec0$765dac40$@gmail.com> <67E78212-F68E-4BD1-946D-F1830EEA2968@darbyshire-bryant.me.uk> <4D01FA8A-E732-49C8-A4E6-8EA453CFA80C@heistp.net> <87d0n6lwgw.fsf@toke.dk> <52F1A35A-9F57-40EC-AF9B-EC6D8BD376BE@darbyshire-bryant.me.uk> <871s3mlsge.fsf@toke.dk> X-Clacks-Overhead: GNU Terry Pratchett Date: Mon, 04 Mar 2019 17:39:47 +0100 Message-ID: <87r2bm386k.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] Using firewall connmarks as tin selectors 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, 04 Mar 2019 16:39:50 -0000 Kevin Darbyshire-Bryant writes: [ ... snipping a bit of context here ... ] >>>>> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp) >>>>> +{ >>>>> + enum ip_conntrack_info ctinfo; >>>>> + struct nf_conn *ct; >>>>> + >>>>> + ct =3D nf_ct_get(skb, &ctinfo); >>>>> + if (!ct) >>>>> + return; >>>>> + >>>>> + ct->mark &=3D 0x80ffffff; >>>>> + ct->mark |=3D (0x40 | dscp) << 24; >>>>=20 >>>> Right, so we *might* have an argument that putting the *tin* into the >>>> fwmark is CAKE's business, but copying over the dscp mark is not >>>> something a qdisc should be doing=E2=80=A6 >>>=20 >>> Why ever not? It=E2=80=99s not the DSCP, it=E2=80=99s a lookup value i= nto the cake >>> priority table, it just happens to look like the DSCP ;-) >>=20 >> If it quacks like a duck=E2=80=A6 > > I honestly don=E2=80=99t know where to go from here. I=E2=80=99m clearly = trying to do > something that the kernel doesn=E2=80=99t want to do. I'm not disputing that what you're trying to do (moving DSCP field into connmark) is useful. I'm just questioning whether CAKE is the right place to do this. I think it would fit better in a TC action; either modify act_connmark, or create a new action to sync fwmarks and DSCP marks... This would both sidestep the whole conntrack dependency issue, and make the same functionality available outside of CAKE (for an HTB-based setup, for instance). > v2 addressing some of the comments attached. Is it best to keep the > in progress patches here or should they be github PRs ? Patches on the mailing list is fine by me, and it seems there are people reading the list, but not github, so let's keep it here for now at least. However, I'll hold off on more detailed comments on the patch until we've resolved the point above. With one exception: > @@ -1661,13 +1695,14 @@ static struct cake_tin_data *cake_select_tin(stru= ct Qdisc *sch, > tin =3D 0; >=20=20 > else if (q->rate_flags & CAKE_FLAG_FWMARK && /* use fw mark */ > - skb->mark && > - skb->mark <=3D q->tin_cnt) > - tin =3D q->tin_order[skb->mark - 1]; > - > - else if (TC_H_MAJ(skb->priority) =3D=3D sch->handle && > - TC_H_MIN(skb->priority) > 0 && > - TC_H_MIN(skb->priority) <=3D q->tin_cnt) > + skb->mark & 0x40000000) { I think there's something odd with this mask? There's only one bit set in it... -Toke