Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux: enable fq_codel for all projects #713

Merged
merged 2 commits into from
May 24, 2017

Conversation

koenkooi
Copy link
Contributor

@koenkooi koenkooi commented Sep 8, 2016

This qdisc aims to eliminate bufferbloat and as a side effect improves
latency during heavy network load. The most notable effect will be a
slightly more responsive ssh session during large downloads.

Systemd will autoselect fq_codel when it's available:

m8s:~ # ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 4096 qdisc noqueue
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP8000> mtu 1500 qdisc fq_codel
qlen 1000
link/ether 82:cd:fa:1b:03:1d brd ff:ff:ff:ff:ff:ff
inet 172.20.0.166/24 brd 172.20.0.255 scope global eth0
valid_lft forever preferred_lft forever
3: wlan0: <NO-CARRIER,BROADCAST,MULTICAST,UP,LOWER_UP8000> mtu 1500
qdisc fq_codel qlen 1000
link/ether 44:2c:05:43:8c:eb brd ff:ff:ff:ff:ff:ff
inet6 fe80::462c:5ff:fe43:8ceb/64 scope link
valid_lft forever preferred_lft forever

On the S905X 100Mbit link it shows no negative effects:

m8s:~ # curl http://beast.local/koen/largefile.mkv --http1.1 > /dev/null
% Total % Received % Xferd Average Speed Time Time Time
% Current
Dload Upload Total Spent Left Speed
7 14.9G 7 1105M 0 0 11.1M 0 0:22:50 0:01:38 0:21:12 11.2M

See https://en.wikipedia.org/wiki/CoDel for more information on CODEL

Signed-off-by: Koen Kooi koen.kooi@linaro.org

@MilhouseVH
Copy link
Contributor

On the S905X 100Mbit link it shows no negative effects

Does it show any positive effects? I'm just wondering what problem this is actually solving - have there been any reports of network issues that this will fix?

@koenkooi
Copy link
Contributor Author

koenkooi commented Sep 8, 2016

I haven't measured it inside LE since netperf and friends aren't installed, but you can notice ping times staying roughly the same during heavy network load (e.g. buffering with readbufferfactor=100).

It doesn't have a big impact on the kodi usecase, but it's a better default the the current one. I wish I could say "It makes internet streaming a lot better", but the biggest problem their will be your modem/router.

So, ehm, yes, a lot of handwaving on my side and no objective measurement :(

@MilhouseVH
Copy link
Contributor

This also only seems to apply to wired ethernet (eth0) as there doesn't appear to be any change for WiFi (wlan0).

I'll include it in my builds tonight, let's see how it goes...

@koenkooi
Copy link
Contributor Author

koenkooi commented Sep 8, 2016

I haven't figured out why some wifi interfaces get it and some don't. These 2 devices run the same .tar (kszaq's s905 tree + kszaq#3), but have different wifi chipsets:

mxIII:~ # ip a | grep wlan
3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop qlen 1000
mxIII:~ # uname -a
Linux mxIII 3.14.29 #1 SMP Thu Sep 8 12:54:32 CEST 2016 aarch64 GNU/Linux
m8s:~ # ip a | grep wlan
3: wlan0: <NO-CARRIER,BROADCAST,MULTICAST,UP,LOWER_UP8000> mtu 1500 qdisc fq_codel qlen 1000
m8s:~ # uname -a
Linux m8s 3.14.29 #1 SMP Thu Sep 8 12:54:32 CEST 2016 aarch64 GNU/Linux

@stefansaraev
Copy link
Contributor

stefansaraev commented Sep 8, 2016

this one does neither good, nor it does harm. pushing it is safe. but no OE/LE user will see any noticeable difference. in practice, it is useless (EDIT: for LE users) :)

@awiouy
Copy link
Collaborator

awiouy commented Sep 8, 2016

Even with heavy downloads, stefansaraev?

@stefansaraev
Copy link
Contributor

even with heavy downloads. if you download something while watching live tv, it's not going to help, at all. you'll have to do more than just having fq_codel active (and LE lacks the tools required to do so, even if they were added, noone would use them anyway) ;)

@lrusak
Copy link
Member

lrusak commented Sep 8, 2016

I'd listen to @stefansaraev here because he is a network guy :)

@stefansaraev
Copy link
Contributor

I dont say you should not merge it. I say it is safe but makes no difference

@MilhouseVH
Copy link
Contributor

Since it makes no appreciable difference I don't realy see a reason to add this, it would just be adding it for the sake of it.

@mcaptur
Copy link
Contributor

mcaptur commented Sep 9, 2016

This might make a difference if link is fully saturated (which is not really the case with a 100mbit or gigabit link on a client) - it would keep latency down.

For the heavy downloads use case you need codel or cake implemented on your internet router as that is most probably the bottleneck introducing latency...

@awiouy
Copy link
Collaborator

awiouy commented Sep 9, 2016

would libevent benefit from this?

@shemminger
Copy link

Setting the sysctl to set the default qdisc works better for this because it replaces all the leaf qdisc as well. It looks like the systemd method can't impact wifi or multi-queue devices.

Use fq_codel as default qdisc

net.core.default_qdisc = fq_codel

@MilhouseVH
Copy link
Contributor

It's not immediately apparent there is a problem that needs solving...

@dtaht
Copy link

dtaht commented Sep 9, 2016

The use for it, for me, on a media center device, is on copying stuff from it while streaming stuff to it. So you get much better/balanced behavior in that scenario.

Now, the ongoing fq_codel wifi work - that's going to make a big difference for stuff streamed to or from a media device over wifi One reason why apple is big on fq_codel'd ecn capability is that it makes forward and rewind very fast, responsive operations. See 16-26 minutes in here:

https://plus.google.com/107942175615993706558/posts/1j8nXtLGZDm

(but that would also require enabling ecn)

...
However we're revising the fq_codel qdisc work to work well on wifi at the mac80211 layer, with nice benefits on both bandwidth and latency.

http://blog.cerowrt.org/post/a_look_back_at_cerowrt_wifi/

Not mainlined, requires driver support, hopefully more people will see results like this and fix the pi3's TERRIBLE wifi implenetation. I'm seeing 500+ms latencies on that at low rates.

...

As for making it the default here, well, it doesn't hurt and can sometimes help. The network will generally always remain more responsive under all loads.

@dtaht
Copy link

dtaht commented Sep 9, 2016

I agree with shemmingers sysctl approach.

@chewitt
Copy link
Member

chewitt commented Sep 10, 2016

no objections from me .. some background reading only throws up positive things

@escalade
Copy link
Contributor

escalade commented Sep 17, 2016

CONFIG_NET_CLS_FLOWER is missing. Other than that it works well (I use the sysctl approach). Been using fq_codel on my router for quite some time, very happy with it.

@chewitt
Copy link
Member

chewitt commented Feb 5, 2017

@koenkooi Can you please rebase/update this PR and include whatever's needed for the sysctl method of enabling. Thanks.

@koenkooi
Copy link
Contributor Author

koenkooi commented Feb 9, 2017

Sure, I'll try to do that over the weekend.

@koenkooi
Copy link
Contributor Author

Rebased the branch with updated kernel configs, sysctl method will be added a bit later.

@chewitt
Copy link
Member

chewitt commented May 14, 2017

@koenkooi needs a rebase and we're still waiting on the sysctl bit :)

@koenkooi
Copy link
Contributor Author

I've put the sysctl.d snippet in the linux package, is that the right place or should I move it to a different package (e.g. systemd)?

# CONFIG_DCB is not set
CONFIG_DNS_RESOLVER=y
# CONFIG_DNS_RESOLVER=y
Copy link
Member

@chewitt chewitt May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is not in other configs / typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to blame 'git rebase', but I think it's a typo. I will drop this change in the next update.

@chewitt
Copy link
Member

chewitt commented May 15, 2017

No objections from me to sysctl in linux package but @lrusak care to comment?

Can you add Slice/Slice3 configs - they were added since the PR was opened, thanks.

@MilhouseVH
Copy link
Contributor

There's no Slice/Slice3 config - they're just sym linked to RPi/RPi2.

@koenkooi
Copy link
Contributor Author

Since slice3 is just a symlink, it does the right thing:

koen@beast:/build/pkg/LibreELEC.tv$ grep FQ_CODEL  ./projects/RPi/devices/Slice3/linux/linux.arm.conf 
CONFIG_NET_SCH_FQ_CODEL=y

@chewitt
Copy link
Member

chewitt commented May 15, 2017

okay, my bad :)

@koenkooi
Copy link
Contributor Author

The CONFIG_DNS_RESOLVER typo has been removed, fix squashed into the original commit.

@MilhouseVH
Copy link
Contributor

MilhouseVH commented May 16, 2017

As already mentioned by @escalade, CONFIG_NET_CLS_FLOWER is missing. Also, CONFIG_NET_CLS_MATCHALL.

I've run make oldconfig on the various projects after applying this PR, and I have the following diffs: http://sprunge.us/XIUf

IMHO it's best to run make oldconfig when submitting config changes, as this saves someone else cleaning up later. :)

I'll include this PR in tonight's test builds.

@koenkooi
Copy link
Contributor Author

Kconfig will autoselect the needed CONFIG_NET_CLS_* options, which I why I haven't included them. And yes, running 'oldconfig' or 'savedefconfig' would be good for patches like this.

@MilhouseVH
Copy link
Contributor

As a general rule I'd rather we "asked" the kernel what options it needs and then added them to the config file, rather than manually pasting what we think it needs (and then have the kernel correct us, or in a worst case situation prompt for an answer).

Although it's often (but not always) cosmetic, these config errors/issues/garbage do increase over time, which makes correcting them more prone to error.

In the diff I posted there are several options that are not supported by some of the project kernels, so I'm not sure why they're being submitted here. And although the CLS options are defaults, there's no reason not to include them - we don't leave out anything else just because it's a default.

If you apply the diff and add an extra commit, it would make this PR go smoother. :)

This qdisc aims to eliminate bufferbloat and as a side effect improves
latency during heavy network load. The most notable effect will be a
slightly more responsive ssh session during large downloads.

Systemd will autoselect fq_codel when it's available:

    m8s:~ # ip a
    1: lo: <LOOPBACK,UP,LOWER_UP> mtu 4096 qdisc noqueue
        link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
        inet 127.0.0.1/8 scope host lo
           valid_lft forever preferred_lft forever
        inet6 ::1/128 scope host
           valid_lft forever preferred_lft forever
    2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP8000> mtu 1500 qdisc fq_codel     qlen 1000
        link/ether 82:cd:fa:1b:03:1d brd ff:ff:ff:ff:ff:ff
        inet 172.20.0.166/24 brd 172.20.0.255 scope global eth0
           valid_lft forever preferred_lft forever
    3: wlan0: <NO-CARRIER,BROADCAST,MULTICAST,UP,LOWER_UP8000> mtu 1500     qdisc fq_codel qlen 1000
        link/ether 44:2c:05:43:8c:eb brd ff:ff:ff:ff:ff:ff
        inet6 fe80::462c:5ff:fe43:8ceb/64 scope link
           valid_lft forever preferred_lft forever

On the S905X 100Mbit link it shows no negative effects:

    m8s:~ # curl http://beast.local/koen/largefile.mkv --http1.1 > /dev/null
      % Total    % Received % Xferd  Average Speed   Time    Time
      % Time
      % Current
                                     Dload  Upload   Total   Spent Left Speed
      7 14.9G    7 1105M    0     0  11.1M      0  0:22:50  0:01:38 0:21:12 11.2M

See https://en.wikipedia.org/wiki/CoDel for more information on CODEL

Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
Copy link
Contributor

@MilhouseVH MilhouseVH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reports of any issues with test builds.

@MilhouseVH MilhouseVH merged commit d5cdb7f into LibreELEC:master May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants