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

Fix cobalt_should_drop() and reduce memory reallocations #137

Closed
wants to merge 2 commits into from

Conversation

inste
Copy link
Contributor

@inste inste commented Jun 23, 2020

Open brand new PR for proper history, see #136

cobalt_should_drop() tries to set CE via INET_ECN_set_ce() call, but this function falsely relies on skb->protocol to determine network header.
Fix this by implementing cobalt_set_ce() helper and employ cake_skb_proto() for proper detection.

Fixes: 3fb9914 ("Retire usage of cobalt.c and merge cobalt functionality elsewhere")
Fixes: 6cc4172 ("Add COBALT core algorithm - combination of simplfied Codel and BLUE.")
Signed-off-by: Ilya Ponetayev <i.ponetaev@ndmsystems.com>
cake_handle_diffserv() tries to linearize mac and network header parts of skb and to make it writable unconditionally.
In some cases it leads to full skb reallocation, which reduces throughput and increases CPU load.
Some measurements of IPv4 forward + NAPT on MIPS router with 580 MHz single-core CPU was conducted.
It appears that on kernel 4.9 skb_try_make_writable() reallocates skb, if skb was allocated in ethernet driver via so-called 'build skb' method
from page cache (it was discovered by strange increase of kmalloc-2048 slab at first).

Obtain DSCP value via read-only skb_header_pointer() call, and leave linearization only for DSCP bleaching or ECN CE setting.

Fixes: 6ff4561 ("sch_cake: Make sure we can write the IP header before changing DSCP bits")
Signed-off-by: Ilya Ponetayev <i.ponetaev@ndmsystems.com>
@tohojo
Copy link
Collaborator

tohojo commented Jun 24, 2020

Merged as 0d67568 (with a few fixups)

@tohojo tohojo closed this Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants