From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) (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 66C2A3BA8E for ; Tue, 21 Aug 2018 17:06:14 -0400 (EDT) Received: by mail-ed1-x52d.google.com with SMTP id h4-v6so89105edi.6 for ; Tue, 21 Aug 2018 14:06:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=heistp.net; s=google; h=from:message-id:mime-version:subject:date:in-reply-to:cc:to :references; bh=OJB7Kyq/UZCHuth1JaR6onWnj/wboLWTDDdRd5TwVKY=; b=Bl4bSt4vTbFLHzdj0fy3JGRUffFml5Gd5mrZU0l3U3j0A6FB5jraAeW96LVAOb6FcJ 7yk7L0VlCORg8lYZgfmxgMWGMtGDcXr8mbKtsYadm72FCCJeHa9NvND0uyXxmIRs2nBT fIsB4Re/FKx5k50xKObl2uy86I1/DdTJoFKIP8kxHE87tZ6hRpCdmDQH3HojyBv+u0V9 U0X+sBILxD9dksPiYzfCS6rHvJwxT7K6D5LKQ/IDZv0HWSEil7Y/hrYe2GTb/gDPG1jX 1NMWpKeZWhZLggPeOq++vGhKwzmCWD2MCB2val0cuz0CD7+DchiXNW5GelIFtuB56yUF 0goA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=OJB7Kyq/UZCHuth1JaR6onWnj/wboLWTDDdRd5TwVKY=; b=Sidy7xPaDU54rIpf+C5I5hxePfjoLc5gsbpTWT0JOhZlEvSVCym1fgjpGrvaniHR6U A/dAKdZCujd8a2Eu4NrauafsYwlUWq1eXgxtv449AbLqdK9JunfR4TYwfKe3QbDfu9r4 NJG2epDxgI/y6SdB43dNAuQGfpMeCv24X0FPNdWlhfnMeBQJBtXhFw/OeBtBCmC72xrS qAD5Mi1tO4ezCm4Z+7BnZnhOszMb1yE7GBTcCxiZPSi8TCW1xYDeXhNC+sQrRB4OLTdL c7DUL5SXy6KxzfXuBJM9RIpDw+Jy69CcamuE1aYQ1wS0gkUFAlQW1HWQir6b9CtS5CNT sNbA== X-Gm-Message-State: APzg51BjbKgxf6j2/6KSiAxaTXpRIS3GM4tmPf1ZHSNJUWYTQgQa4qMk CO4sxKAHR0faK+pkMeL2XhvbHw== X-Google-Smtp-Source: ANB0VdYlF/RO4xVbW3zby1ncCMwxEqU+rqHfDrcBVieokUYDoJSPWhOjs/ij2XNzykLDCiYHVAsgNQ== X-Received: by 2002:a50:9817:: with SMTP id g23-v6mr5644828edb.277.1534885573533; Tue, 21 Aug 2018 14:06:13 -0700 (PDT) Received: from tron.luk.heistp.net (h-1169.lbcfree.net. [185.193.85.130]) by smtp.gmail.com with ESMTPSA id l30-v6sm37035edc.70.2018.08.21.14.06.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Aug 2018 14:06:12 -0700 (PDT) From: Pete Heist Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_FA2A2A3A-CCF1-4A1E-8A43-3E685EFDFED0" Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Date: Tue, 21 Aug 2018 23:06:11 +0200 In-Reply-To: <87d0uc9d2f.fsf@toke.dk> Cc: Jonathan Morton , Cake List To: =?utf-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= References: <87h8jze5hk.fsf@toke.dk> <85C60B2F-78D0-4AEE-871C-BB637785BF62@gmail.com> <4D28C453-5378-4A5B-9E05-874F36C4DB30@gmail.com> <878t5aedj5.fsf@toke.dk> <87d0uc9d2f.fsf@toke.dk> X-Mailer: Apple Mail (2.3445.9.1) Subject: Re: [Cake] issue with Cake and bpf filter 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: Tue, 21 Aug 2018 21:06:14 -0000 --Apple-Mail=_FA2A2A3A-CCF1-4A1E-8A43-3E685EFDFED0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Aug 21, 2018, at 1:25 PM, Toke H=C3=B8iland-J=C3=B8rgensen = wrote: >=20 >>> The next simplest fix is to ignore the flow ID override unless we're >>> in "flows" mode. We can then make valid assumptions about what = should >>> go into the host tables. >>>=20 >>> The *right* fix, if we want to maximise functionality, would be to >>> pass the result struct by reference into cake_hash(), where it can >>> override the *host* IDs (not the flow ID). Users can then choose >>> between using the override as a flow ID (by setting "hosts" mode >>> instead of "flows"), or retaining the default host-isolation = semantics >>> with a revised definition of "host". >>=20 >> Ah, making it possible to override both host and flow mode is a great >> idea! I guess we could use the major/minor distinction in the class = to >> steer this. I'll see if I can't integrate this. >=20 > So, I implemented this; in the latest commit on github it is again > possible to override the flow hashing by setting the class ID with a = TC > filter; and the host hash can be overridden by setting the major = number > of the class ID. In my testing the hangs from before are gone, but if > anyone else wants to test, please do! >=20 > I'll write up a description of the filter overrides in the man page, = and > submit the change upstream as well... Well that=E2=80=99s good timing for me as I=E2=80=99m wrapping up a = small utility/eBPF to classify an arbitrary username to either MAC or = IP. Here=E2=80=99s the work in progress, which is not done yet as flow = fairness is still under construction, and I haven=E2=80=99t gotten my = IPv6 support to pass the rather stubborn eBPF verifier: = https://github.com/heistp/tc-users With your new code Toke: - I so far haven=E2=80=99t seen my VM either crash or suddenly fill its = disk with logs, which is a bonus. :) - With the new major/minor ID distinction, I=E2=80=99d probably use = major for the user and minor for the flow hash? Another thing I haven=E2=80=99t looked into yet is that when fq_codel is = the qdisc, the eBPF action is only called "once in a while=E2=80=9D = (start of a new flow?) With cake it=E2=80=99s called for every single = packet, which is what I expected to happen, but very different behavior. Lastly, if anyone has time to review even just a little code for what is = or is not good or idiomatic C, post an issue and I=E2=80=99d appreciate = it. Yes, I yield to the =E2=80=98goto=E2=80=99 proponents when it comes = to error handling and resource de-allocation. :) Pete --Apple-Mail=_FA2A2A3A-CCF1-4A1E-8A43-3E685EFDFED0 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
On = Aug 21, 2018, at 1:25 PM, Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> = wrote:

The next simplest fix is to ignore the flow ID override = unless we're
in "flows" mode. We can then make valid = assumptions about what should
go into the host tables.

The *right* fix, if we want to maximise = functionality, would be to
pass the result struct by = reference into cake_hash(), where it can
override the = *host* IDs (not the flow ID). Users can then choose
between = using the override as a flow ID (by setting "hosts" mode
instead of "flows"), or retaining the default host-isolation = semantics
with a revised definition of "host".

Ah, making it possible to = override both host and flow mode is a great
idea! I guess = we could use the major/minor distinction in the class to
steer this. I'll see if I can't integrate this.

So, I implemented this; in the = latest commit on github it is again
possible to override = the flow hashing by setting the class ID with a TC
filter; = and the host hash can be overridden by setting the major number
of the class ID. In my testing the hangs from before are = gone, but if
anyone else wants to test, please do!

I'll write up a description of the filter = overrides in the man page, and
submit the change upstream = as well...

Well = that=E2=80=99s good timing for me as I=E2=80=99m wrapping up a small = utility/eBPF to classify an arbitrary username to either MAC or IP. = Here=E2=80=99s the work in progress, which is not done yet as flow = fairness is still under construction, and I haven=E2=80=99t gotten my = IPv6 support to pass the rather stubborn eBPF verifier: https://github.com/heistp/tc-users
With your new code Toke:
- I so far haven=E2=80=99t seen my VM either crash or = suddenly fill its disk with logs, which is a bonus. :)
- With the new major/minor ID distinction, I=E2=80=99d = probably use major for the user and minor for the flow hash?

Another thing I = haven=E2=80=99t looked into yet is that when fq_codel is the qdisc, the = eBPF action is only called "once in a while=E2=80=9D (start of a new = flow?) With cake it=E2=80=99s called for every single packet, which is = what I expected to happen, but very different behavior.

Lastly, if anyone = has time to review even just a little code for what is or is not good or = idiomatic C, post an issue and I=E2=80=99d appreciate it. Yes, I yield = to the =E2=80=98goto=E2=80=99 proponents when it comes to error handling = and resource de-allocation. :)

Pete

= --Apple-Mail=_FA2A2A3A-CCF1-4A1E-8A43-3E685EFDFED0--