On 8 January 2017 at 07:18, Paul Blakey <[email protected]> wrote: > > On 05/01/2017 02:11, Joe Stringer wrote: > >> On 25 December 2016 at 03:39, Paul Blakey <[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 natrual >>> 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. >>> >>> changes from V1 >>> - Added generic netdev flow offloads API. >>> - Implemented relevant flow API in netdev-linux (and netdev-vport). >>> - Added a other_config hw-offload option to enable offloading >>> (defaults to false). >>> - Fixed coding style to conform with OVS. >>> - Policy removed for now. (Will be discussed how best implemented >>> later). >> >> Hi Paul, >> >> Happy holidays to you too ;) > > Hi :) > Thanks for your thorough review, We are working on resubmitting the next > patch set and I'll > try and reply as much as I can right now, but this is a big review, so I'll > split this to several replies. >> >> >> A few high level comments: >> * Overall the patchset is looking in better state than previously. >> * That said, on platforms other than your specific test environment >> OVS fails to build. >> * netdev-vport seems to have acquired platform-specific code > > Thanks, We've seen your links for travis-ci and app veyor, we'll create an > account and fix it accordingly. >> >> * I don't think that ovs-dpctl should read the database. One of the >> nice things about ovs-dpctl is that it is currently standalone, ie >> doesn't require anything else to be running to be able to query the >> state of the kernel datapath. What are you trying to achieve with >> this? Perhaps you can extend the ovs-appctl dpctl/... commands instead >> to provide the functionality you need? > > We have two config options that isn't saved with the kernel datapath (and > dumped back on dpif open): > if offload is enabled to know if we even need to dump from netdevs or not > for dump-flows cmd (I guess we could > just dump the netdev anyway) and for add-flow, and tc flower flag > skip_hw/skip_sw that is also needed for add-flow cmd, > so we can know what flag we be used while inserting to flower if offloading > is enabled and possible.
I discussed this with a few people offline and it seems that the most reasonable way to handle it for dump is just to dump from netdevs anyway. You can fetch the list of relevant ports from OVS datapath in kernel, then only dump those ports. For adding and deleting flows, using ovs-dpctl is not really supported, it's primarily for debug. ovs-vswitchd is going to override that behaviour anyway - if you add a flow, it may delete it; if you delete a flow, it may re-add it. I suggest that for debugging purposes the default behaviour is to install into OVS, but if you want to put it into TC sw/hw, provide an additional flag on the commandline. If you want ovs-vswitchd to decide how to install the flow, you can always use "ovs-appctl dpctl/add-flow ..." and the command will go via ovs-vswitchd, which will make the offloading decision based on the same logic as it would when handling regular upcalls. >> * It would be good to see at least some basic system tests ala >> tests/system-traffic.at or tests/system-ovn.at. Maybe there could be >> tests/system-tc.at that uses SKIP_HW to show that flows can be >> installed and that they work. > > Sure. >> >> * I think it would be great to get some ability to see what's >> happening with flow offloads; eg "ovs-appctl dpctl/show-hw-offloads" >> that could give you some idea of how many flows are being offloaded > > You mean a number/stats or actually dump only offloaded flows? At the time, I was thinking just stats. However I think it would also be useful to have some flag for ovs-dpctl that chooses which flows to fetch. Something like: ovs-dpctl dump-flows: Dump all flows from both OVS and TC (flower only). ovs-dpctl dump-flows --only-flower ovs-dpctl dump-flows --only-ovs Each flow when printed should also indicate in some way where it came from. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
