> -----Original Message----- > From: Yuanhan Liu [mailto:y...@fridaylinux.org] > Sent: Wednesday, February 7, 2018 9:37 AM > To: Stokes, Ian <ian.sto...@intel.com> > Cc: Flavio Leitner <f...@sysclose.org>; d...@openvswitch.org; Simon Horman > <simon.hor...@netronome.com>; Olga Shern <ol...@mellanox.com>; Shahaf > Shuler <shah...@mellanox.com> > Subject: Re: [ovs-dev] [PATCH v7 0/6] OVS-DPDK flow offload with rte_flow > > On Wed, Feb 07, 2018 at 09:24:52AM +0000, Stokes, Ian wrote: > > > -----Original Message----- > > > From: Yuanhan Liu [mailto:y...@fridaylinux.org] > > > Sent: Wednesday, February 7, 2018 2:12 AM > > > To: Stokes, Ian <ian.sto...@intel.com> > > > Cc: Flavio Leitner <f...@sysclose.org>; d...@openvswitch.org; Simon > > > Horman <simon.hor...@netronome.com>; Olga Shern > > > <ol...@mellanox.com>; Shahaf Shuler <shah...@mellanox.com> > > > Subject: Re: [ovs-dev] [PATCH v7 0/6] OVS-DPDK flow offload with > > > rte_flow > > > > > > On Tue, Feb 06, 2018 at 09:34:36PM +0000, Stokes, Ian wrote: > > > > > -----Original Message----- > > > > > From: Yuanhan Liu [mailto:y...@fridaylinux.org] > > > > > Sent: Tuesday, February 6, 2018 9:10 AM > > > > > To: Flavio Leitner <f...@sysclose.org>; Stokes, Ian > > > > > <ian.sto...@intel.com> > > > > > Cc: d...@openvswitch.org; Simon Horman > > > > > <simon.hor...@netronome.com> > > > > > Subject: Re: [ovs-dev] [PATCH v7 0/6] OVS-DPDK flow offload with > > > > > rte_flow > > > > > > > > > > On Thu, Feb 01, 2018 at 01:35:15AM -0200, Flavio Leitner wrote: > > > > > > On Mon, Jan 29, 2018 at 02:59:42PM +0800, Yuanhan Liu wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Here is a joint work from Mellanox and Napatech, to enable > > > > > > > the flow hw offload with the DPDK generic flow interface > (rte_flow). > > > > > > > > > > > > > > The basic idea is to associate the flow with a mark id (a > > > > > > > unit32_t > > > > > number). > > > > > > > Later, we then get the flow directly from the mark id, which > > > > > > > could bypass some heavy CPU operations, including but not > > > > > > > limiting to mini flow extract, emc lookup, dpcls lookup, etc. > > > > > > > > > > > > > > The association is done with CMAP in patch 1. The CPU > > > > > > > workload bypassing is done in patch 2. The flow offload is > > > > > > > done in patch 3, which mainly does two things: > > > > > > > > > > > > > > - translate the ovs match to DPDK rte flow patterns > > > > > > > - bind those patterns with a RSS + MARK action. > > > > > > > > > > > > > > Patch 5 makes the offload work happen in another thread, for > > > > > > > leaving the datapath as light as possible. > > > > > > > > > > > > > > A PHY-PHY forwarding with 1000 mega flows > > > > > > > (udp,tp_src=1000-1999) and > > > > > > > 1 million streams (tp_src=1000-1999, tp_dst=2000-2999) show > > > > > > > more than 260% performance boost. > > > > > > > > > > > > > > Note that it's disabled by default, which can be enabled by: > > > > > > > > > > > > Hi, > > > > > > > > > > > > First of all, thanks for working on this feature. > > > > > > > > > > > > I have some general comments I'd like to discuss before going > > > > > > deeper on it. > > > > > > > > > > > > The documentation is too simple. It should mention the HW > > > > > > requirements in order to use this feature. > > > > > > > > > > It listed the NICs that support this feature. > > > > > > > > > > > Also some important limitations, like no support for IP > > > > > > frags, MPLS or conntrack, for instance. > > > > > > > > > > Yes, that could be added. > > > > > > > > > > > It seems it would be possible to leave the HW offloading code > > > > > > outside of dpif-netdev.c which is quite long already. I hope > > > > > > it will improve isolation and code clarity too. > > > > > > > > > > Yes. My thoughts was we can do it later, when we are going to > > > > > add some other flow models (say, full offload) in future. > > > > > Moreover, it seems that it's a custom in OVS to have long source > > > > > code files. I was just following it. > > > > > > > > > > But I hve no objections to break it. Ian, should I break it now? > > > > > What's your plan to merge it? > > > > > > > > Hi Yuanhan, I believe I gave the same feedback on the first review > > > > pass > > > I did. Previously I was happy to hold off on this work in order to > > > make it into the 2.9 release but as that window has passed I think > > > it makes sense to break it out now. It will save the refactor work > later. > > > > > > So it won't in ovs 2.9 release? > > > > No, the code freeze was the 31st of January for new features. Only bug > fixes are accepted for the remaining 2.9 window. This would be part of > the 2.10 release. > > I thought we all come an agreement that this will be part of 2.9 release > as a experimental feature. What made we missed it?
So there was agreement the feature would be experimental when it is upstreamed to master (not just for OVS 2.9). Unfortunately it missed the 2.9 deadline due to the lack of reviews external to authors within community. I was hoping to review it myself in the run up to the 31st however a separate issue relating to dpdk mempools took priority as it was a blocking issue for users upgrading from OVS 2.8 to OVS 2.9. It was raised by the community and took priority. I've had time only to reviewed the first patch and the documentation patch of the series at this point. I understand it's experimental but I think it's important that it's still reviewed and feedback given before its introduction to OVS. AFAIK there's still issues with compilation for sparse etc. which was breaking the OVS travis build. This alone would keep it from being upstreamed at this point. I'm planning to look at review the rest of this set later this week and providing more feedback. Ian > > Thanks. > > --yliu _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
