On 5/4/22 10:51, Stokes, Ian wrote: >> Hello Kumar, >> >> On Tue, Apr 12, 2022 at 4:13 PM Kumar Amber <[email protected]> >> wrote: >>> >>> For packets which don't already have a hash calculated, >>> miniflow_hash_5tuple() calculates the hash of a packet >>> using the previously built miniflow. >>> >>> This commit adds IPv4 profile specific hashing which >>> uses fixed offsets into the packet to improve hashing >>> performance. >> >> The code lgtm. >> >> I did not check AVX512 implementation, but as for the scalar code, I >> did some benchmarks and I see: >> - a limited, yet consistent accross the runs, -2% (for 1c/2t) change >> in maximum throughput when doing a loopback test with one physical >> port, >> - a +6% when doing a pvp test (physical to vhost, looped back with testpmd), >> >> The drop is due to the additional branch in miniflow_extract. >> I don't see a way to enhance this, I tried different forms of >> inline/no inline, likely/unlikely but it did not help. >> >> Seeing how PVP gets better, I think the performance drop in the >> physical only usecase is acceptable. >> > > Thanks for the review David, much appreciated. > > @Ilya Maximets are there any other blockers at this point on this series? I > think this is good to go from my side also but wanted to check with yourself > if outstanding issues you've flagged are resolved?
The code changes looks fine, but I didn't test this version. I'm not sure if we need a NEWS entry for this change as it's only sort of user-visible. In any case, the current version of the NEWS entry isn't really informative as it's unclear for someone, who doesn't know the code, what does it mean. Even from the developer's point of view it kind of lost its meaning in v10. What do you think? Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
