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

Integrated PartitionSort into Lagopus #132

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

marcusj6
Copy link

@marcusj6 marcusj6 commented Oct 17, 2017

We have found that Sorrachai Yingchareonthawornchai's PartitionSort outperforms all other classifiers in build time when incremental update is used. In addition, it classifies faster than all others when many fields are used. Furthermore, when compared against Mbtree and Flowinfo, it was found that Mbtree and Flowinfo classify some packets incorrectly. These results can be seen here. Therefore, PartitionSort is a viable alternative to the current classifiers available in Lagopus. We have integrated PartitionSort into Lagopus as an optional classifier. It can be enabled in configure with option --enable-psort. There are two tests included: functional_test and performance_test. Both are located in lagopus/test/dataplane/benchmark and require PartitionSort to be enabled to function.

@hirokazutakahashi
Copy link
Contributor

Thank you! I'll test it.

@marcusj6
Copy link
Author

After some more thorough testing, I found that the PartitionSort search criteria differs from THTable in that it selects the highest priority matching flow rather than lowest when multiple flows have identical match fields. This is a minor issue that causes PartitionSort to classify some packets differently from THTable (it chooses a different matching flow). I will make a second pull request soon in which the search criteria is corrected.

@marcusj6
Copy link
Author

The search criteria issue has been corrected, PartitionSort should be performing as expected now

@hibitomo
Copy link
Contributor

hibitomo commented Aug 3, 2018

Hi marcusj6,

Sorry, late reply.
I tested this pull requests, but I failed the compile in commit 7e3b87e.
And, b240e22 and 393b3b0 are failed all tests of ryu-certification.

Thanks.

@marcusj6
Copy link
Author

marcusj6 commented Sep 4, 2018

Hi,

I fixed the compile issue in the next commit, it was a result of a missing conditional group. I reran all of the tests with make check, none of them failed on my machine. When I set the configuration file to use PartitionSort, I use the following command: ./configure --enable-dpdk --enable-developer --enable-psort. The final command is necessary to use PartitionSort. Is it failing when you enable it? Do I need to do the ryu-certification with separate tests?

Best regards,
Jacob Marcus

@Nic30
Copy link

Nic30 commented Jan 21, 2019

Hello,

what is the result of the integration?

I am trying to create variant of @marcusj6 algorithm for FPGA + HBM memory.
@marcusj6 are you still interested in PartitionSort development?

I did complete rewrite and used boost rbtree, parallelized some parts of alg. and used dynamic programming to predict the correct tree to insert rule to.
Also I added support for hashing of some parts of the tree.
The problem I am facing is the base idea of the PartitionSort as the field order is global for a tree it greatly reduces the memory efficiency of the decision tree.
But maybe I just misunderstand something.

@marcusj6
Copy link
Author

marcusj6 commented Feb 1, 2019

Hi Nic30,

PartitionSort has been fully integrated into the switch. We are currently awaiting more information about the certification tests that it failed.

We are still interested in PartitionSort development.

The field order is global for the tree to simplify construction and update. The field order chosen must totally order a ruleset, so finding an order that will work is relatively expensive. Since the field order is global, there is no need to recompute the field order when a node is inserted or deleted (unless the number of rules is small).

@Nic30
Copy link

Nic30 commented Feb 1, 2019

Hi @marcusj6,

in DPDK they are using SIMD to achieve much faster evaluation of the tree https://github.com/DPDK/dpdk/blob/master/lib/librte_acl/acl_run_avx2.h

Do you know about anyone who tried to do same thing with PartitionSort?
Also does the PartitionSort really brings the performance improvements? Currently I am trying to do some benchmark to compare it with DPDK librte_acl and ovs tuple space search and it looks same or worse than librte_acl. But maybe it is just my bad implementation or rule set or the generated traffic.

@Nic30
Copy link

Nic30 commented Feb 1, 2019

Also are you thinking about using lets say libboost? I mean there is plenty of red-black tree related code which is already written in other libraries and which is quite complex and large.

@marcusj6
Copy link
Author

marcusj6 commented Feb 5, 2019

Hi @Nic30 ,

I have not seen anyone attempt to use SIMD with PartitionSort. It could be done by replacing the red-black tree with a B+-tree and using a variant of k-ary search on the keys. The red-black tree was used for simplicity, not necessarily because it was the best performing option. A B+-Tree would probably have better caching behavior too. Libboost is a good option for improving the performance.

PartitionSort outperformed tuple space search in our benchmark, but we did not compare it with DPDK librte_acl. What is the benchmark you are using?

@Nic30
Copy link

Nic30 commented Feb 6, 2019

@marcusj6

B+tree

I was also thinking about it, few months back It was not suitable for hardware implementation. But now for the software, ...
There are plenty of efficient aproximately balance trees, we need some library where we can test all the tree algs. without much changes in code. I was thinking that I just write such a library, but there is always N+1 problems...

benchmark

I do have some rule sets generated by classbench-ng and I am using similar code as used in in PartitionSort repo I did rewrite it and added the DPDK librte_acl classifier.

I am not using any specific virtual switch or any other real word app.
(But now I am experimenting with OvS.)

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

4 participants