AGW: pipelined: use pyroute2 to manage TC queue and filters#5240
Merged
pshelar merged 1 commit intomagma:masterfrom Mar 3, 2021
Merged
AGW: pipelined: use pyroute2 to manage TC queue and filters#5240pshelar merged 1 commit intomagma:masterfrom
pshelar merged 1 commit intomagma:masterfrom
Conversation
koolzz
approved these changes
Mar 2, 2021
Contributor
koolzz
left a comment
koolzz
left a comment
There was a problem hiding this comment.
those stress test numbers look gooooooooooood
| self._uplink = config['nat_iface'] | ||
| self._downlink = config['enodeb_iface'] | ||
| self._max_rate = config["qos"]["max_rate"] | ||
| self._enable_pyroute2 = config["qos"].get('enable_pyroute2', False) |
Contributor
There was a problem hiding this comment.
should this be true by default?
Contributor
Author
There was a problem hiding this comment.
I still want to make sure all tests works fine before making it default
| def __init__(self): | ||
| self._ipr = IPRoute() | ||
| self._iface_if_index = {} | ||
| LOG.info("initialized") |
Contributor
Author
There was a problem hiding this comment.
This msg is logs shows which TC backend implementation we are using.
| ret = self._ipr.tc("add-class", "htb", if_index, | ||
| htb_queue, parent=parent_qid, | ||
| rate=str(rate).lower(), ceil=max_bw, prio=1) | ||
| LOG.debug("Return: %s", ret) |
Contributor
Author
There was a problem hiding this comment.
yes, it returns class state from kernel.
Contributor
There was a problem hiding this comment.
yeah I just meant expand log msg so its clear
Contributor
Author
There was a problem hiding this comment.
logger has prefix set. So in log it looks like:
Mar 1 21:59:15 magma-dev pipelined[11667]: INFO:pipelined.qos.tc_pyroute2:initialized
| try: | ||
| if_index = self._get_if_index(iface) | ||
|
|
||
| class_id = int(0x10000) | int(qid, 16) |
Contributor
There was a problem hiding this comment.
I see 0x10000 used a bunch, make it a const?
Codecov Report
@@ Coverage Diff @@
## master #5240 +/- ##
=======================================
Coverage 63.61% 63.61%
=======================================
Files 301 301
Lines 20300 20300
=======================================
+ Hits 12913 12914 +1
+ Misses 5486 5485 -1
Partials 1901 1901
Continue to review full report at Codecov.
|
pshelar
force-pushed
the
pyroute2-ops3
branch
2 times, most recently
from
3039e61 to
7a26d76
Compare
pshelar
force-pushed
the
pyroute2-ops3
branch
2 times, most recently
from
29bedeb to
1a5f1cd
Compare
tmdzk
requested changes
Mar 2, 2021
| "libtins-dev" # required for Connection tracker | ||
| "libmnl-dev" # required for Connection tracker | ||
| "getenvoy-envoy" # for envoy dep | ||
| "python3-pyroute2" |
Collaborator
There was a problem hiding this comment.
As It's a python dep can you add it to the setup.py instead of here?
tmdzk
approved these changes
Mar 2, 2021
ardzoht
approved these changes
Mar 3, 2021
Following commit adds second implementation for executing TC commands. This is using netlink socket rather than executing `tc` commands. I have seen good improvement over current master. following numbers are using `pipelined_cli.py enforcement stress_test_grpc` With pyroute2 ------------ Starting attaches Finished 600 attaches in 9.850383 seconds Actual attach rate = 61 UEs per sec Starting detaches Finished 600 detaches in 6.381709 seconds with current master ------------------- Starting attaches Finished 600 attaches in 22.11333 seconds Actual attach rate = 27 UEs per sec Starting detaches Finished 600 detaches in 6.360227 seconds Signed-off-by: Pravin B Shelar <[email protected]>
rsarwad
pushed a commit
to rsarwad/magma
that referenced
this pull request
Mar 3, 2021
Following commit adds second implementation for executing TC commands. This is using netlink socket rather than executing `tc` commands. I have seen good improvement over current master. following numbers are using `pipelined_cli.py enforcement stress_test_grpc` With pyroute2 ------------ Starting attaches Finished 600 attaches in 9.850383 seconds Actual attach rate = 61 UEs per sec Starting detaches Finished 600 detaches in 6.381709 seconds with current master ------------------- Starting attaches Finished 600 attaches in 22.11333 seconds Actual attach rate = 27 UEs per sec Starting detaches Finished 600 detaches in 6.360227 seconds Signed-off-by: Pravin B Shelar <[email protected]>
themarwhal
pushed a commit
that referenced
this pull request
Mar 3, 2021
Following commit adds second implementation for executing TC commands. This is using netlink socket rather than executing `tc` commands. I have seen good improvement over current master. following numbers are using `pipelined_cli.py enforcement stress_test_grpc` With pyroute2 ------------ Starting attaches Finished 600 attaches in 9.850383 seconds Actual attach rate = 61 UEs per sec Starting detaches Finished 600 detaches in 6.381709 seconds with current master ------------------- Starting attaches Finished 600 attaches in 22.11333 seconds Actual attach rate = 27 UEs per sec Starting detaches Finished 600 detaches in 6.360227 seconds Signed-off-by: Pravin B Shelar <[email protected]>
chandra-77
pushed a commit
to chandra-77/magma
that referenced
this pull request
Mar 30, 2021
Following commit adds second implementation for executing TC commands. This is using netlink socket rather than executing `tc` commands. I have seen good improvement over current master. following numbers are using `pipelined_cli.py enforcement stress_test_grpc` With pyroute2 ------------ Starting attaches Finished 600 attaches in 9.850383 seconds Actual attach rate = 61 UEs per sec Starting detaches Finished 600 detaches in 6.381709 seconds with current master ------------------- Starting attaches Finished 600 attaches in 22.11333 seconds Actual attach rate = 27 UEs per sec Starting detaches Finished 600 detaches in 6.360227 seconds Signed-off-by: Pravin B Shelar <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following commit adds second implementation for executing
TC commands. This is using netlink socket rather than executing
tccommands. I have seen good improvement over current master.following numbers are using
pipelined_cli.py enforcement stress_test_grpcWith pyroute2
Starting attaches
Finished 600 attaches in 9.850383 seconds
Actual attach rate = 61 UEs per sec
Starting detaches
Finished 600 detaches in 6.381709 seconds
with current master
Starting attaches
Finished 600 attaches in 22.11333 seconds
Actual attach rate = 27 UEs per sec
Starting detaches
Finished 600 detaches in 6.360227 seconds
Signed-off-by: Pravin B Shelar [email protected]
Summary
Test Plan
make testAdditional Information