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

Are the tc prio qdisc flowid's correct in the tuning guide? #11469

Closed
ae6rt opened this issue Dec 19, 2019 · 8 comments
Closed

Are the tc prio qdisc flowid's correct in the tuning guide? #11469

ae6rt opened this issue Dec 19, 2019 · 8 comments

Comments

@ae6rt
Copy link
Contributor

ae6rt commented Dec 19, 2019

In the tuning guide Network Section the flowid value 1:1 is stated to be the same for peer traffic (port 2380) as it is for client traffic (port 2379)

tc qdisc add dev eth0 root handle 1: prio bands 3
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 match ip sport 2380 0xffff flowid 1:1
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 match ip dport 2380 0xffff flowid 1:1
tc filter add dev eth0 parent 1: protocol ip prio 2 u32 match ip sport 2379 0xffff flowid 1:1
tc filter add dev eth0 parent 1: protocol ip prio 2 u32 match ip dport 2379 0xffff flowid 1:1

If the goal of the prio qdisc configuration is to give priority to peer traffic on port 2380, why would the client traffic on port 2379 not be classified to band 1:2? My reading of the man page for tc-prio says that band 0 (parent:1) is dequeued first, followed by band 1 (parent:2), where parent is 1 from the qdisc creation itself.

It is true that the filter rules themselves are given priorities (prio 1 and prio 2) consistent with preferring peer over client traffic, but this is to impose filter evaluation order, and seems orthogonal to how packets are prioritized if peer and client packets are both queued for egress.

@jpbetz
Copy link
Contributor

jpbetz commented Mar 26, 2020

@heyitsanthony do you recall what the original intent was here?

@ae6rt My reading of the documentation agrees with what you concluded. It might also be worth documenting what happens to all other traffic. Should 2380 be highest priority, 2379 second highest, and all other traffic lower priority than that? If so we should document that rule as well.

I'll wait for some feedback and then send out a PR for the documentation change.

@jpbetz
Copy link
Contributor

jpbetz commented Mar 26, 2020

I'm thinking of something like this as the replacement documentation:

tc qdisc add dev eth0 root handle 1: prio bands 3
# Prioritize peer traffic
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 match ip sport 2380 0xffff flowid 1:1
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 match ip dport 2380 0xffff flowid 1:1
# Optional: Set client traffic at next highest priority.
tc filter add dev eth0 parent 1: protocol ip prio 2 u32 match ip sport 2379 0xffff flowid 1:2
tc filter add dev eth0 parent 1: protocol ip prio 2 u32 match ip dport 2379 0xffff flowid 1:2
# Set all other traffic at a lower priority
tc filter add dev eth0 parent 1: protocol ip prio 3 flowid 1:3

@stale
Copy link

stale bot commented Jun 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 24, 2020
@ae6rt
Copy link
Contributor Author

ae6rt commented Jun 24, 2020

Curious if there was any word from the original author of the tc tuning advice?

@stale stale bot removed the stale label Jun 24, 2020
@stale
Copy link

stale bot commented Sep 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 22, 2020
@ae6rt
Copy link
Contributor Author

ae6rt commented Sep 22, 2020

Any word on this @jpbetz or @heyitsanthony ?

Thanks.

@stale stale bot removed the stale label Sep 22, 2020
@jpbetz
Copy link
Contributor

jpbetz commented Sep 23, 2020

I didn't have any thoughts beyond what I mentioned in #11469 (comment). If anyone wants to double check or improve and open a PR to fix the documentation I'd be happy to review.

@stale
Copy link

stale bot commented Dec 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 22, 2020
@stale stale bot closed this as completed Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants