On 22 March 2017 at 04:10, Roi Dayan <[email protected]> wrote: > This patch series introduces rule offload functionality to dpif-netlink > via netdev ports new flow offloading API. The user can specify whether to > enable rule offloading or not via OVS configuration. Netdev providers > are able to implement netdev flow offload API in order to offload rules. > > This patch series also implements one offload scheme for netdev-linux, > using TC flower classifier, which was chosen because its sort of natural > to state OVS DP rules for this classifier. However, the code can be > extended to support other classifiers such as U32, eBPF, etc which support > offload as well. > > The use-case we are currently addressing is the newly sriov switchdev mode > in the Linux kernel which was introduced in version 4.8 [1][2]. > This series was tested against sriov vfs vports representors of the > Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver. > > > V4->V5: > - Fix compat > - Fix VXLAN IPv6 tunnel matching > - Fix order of actions in dump flows > - Update ovs-dpctl man page about the addtion of type to dump-flows > > Travis > https://travis-ci.org/roidayan/ovs/builds/213735371 > AppVeyor > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18
Hi Roi, This series does not compile on the OVS master: https://travis-ci.org/joestringer/openvswitch/builds/214039943 I ran the make check-offloads tests on a recent net-next kernel and it failed, output was not as expected: ../../tests/system-offloaded-traffic.at:54: ovs-appctl dpctl/dump-flows | grep "eth_type(0x0800)" | sed -e 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort --- - 2017-03-22 16:43:37.598689692 -0700 +++ /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout 2017-03-22 16:43:37.595628000 -0700 @@ -1,3 +1,3 @@ -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s, actions:output -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s, actions:output +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882, used:0.001s, actions:output +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882, used:0.001s, actions:output Did you end up allowing other_config:hw-offload to be configured at runtime? The tests seem to be assuming this, but the documentation still says that OVS must be restarted to enable offloads. The testsuite is an encouraging sign. I can only imagine that it's not attempting to comprehensively cover OVS flow translation to the tc flower API, because of its simplicity. Presumably you intend to improve this over time though? At a high level, the functionality isn't particularly compelling at this stage. OVS has a huge level of programmability, and the limits applied in this patch restrict the offloads to quite a small subset of use cases. That said, I believe I've mentioned off-list a couple of times that if there is broad interest in the OVS community regarding this series, and that interest is shown (for instance, by people trying out the patches, providing review, etc) then that's an encouraging sign that this feature is providing useful functionality---and therefore should be merged. For reference, the kinds of use cases that I think could make this series more compelling include stateful connection tracking and tunnel termination, in particular with TLV support (for Geneve, NSH). The biggest concern from a maintainership point of view is the potential burden on reviews and support that this may introduce. Your approach of making this disabled by default and configurable in the database makes this easier, as this restricts the potential impact on regular users. I've heard arguments on both sides of this, but effectively if over time the usefulness is low and maintenance burden is high, we'd be inclined to remove this again; rather, if it becomes more useful over time and we get active involvement from the community for ongoing patch review and assisting users that run into trouble, then that would make the maintainers job easier. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
