Hi Jesse, Thank you for looking into the patch. I will send out v2 patch after incorporating your comments.
Regards _Sugesh > -----Original Message----- > From: Jesse Gross [mailto:je...@kernel.org] > Sent: Monday, April 11, 2016 5:33 PM > To: Chandran, Sugesh <sugesh.chand...@intel.com> > Cc: ovs dev <dev@openvswitch.org> > Subject: Re: [ovs-dev] [PATCH] tunneling: Improving VxLAN tunneling > performance using DPDK Rx checksum offloading feature. > > On Mon, Apr 11, 2016 at 2:52 AM, Sugesh Chandran > <sugesh.chand...@intel.com> wrote: > > Optimizing VxLAN tunneling performance in userspace datapath by > > offloading the rx checksum validation on tunnel packets to the NIC when it > is supported. > > > > Signed-off-by: Sugesh Chandran <sugesh.chand...@intel.com> > > Thanks for working on this. I think there is a lot of room for OVS DPDK to > take > advantage of common NIC offloads. > > I have some high level comments: > > * I noticed that you've been following up you patches with a second message > with performance numbers. I think it would be better to just include this in > the patch description since that's what will become the commit message. > Even if the numbers might change over time, it's useful to have a reference > to what the expected performance benefit was at the time the patch was > authored. Related to this, it would also be useful to have more context on > the benchmark numbers that you are providing. For example, I'm guessing > that the test was run using VXLAN with outer UDP checksums? What NIC was > used? [Sugesh] Yes, its VxLAN with outer UDP checksums. I have used Intel Niantic 10G NICs. The test setup is as follows NIC <--> OVS<-->NIC 64 Byte UDP stream (14 million) --> 114 Byte VxLAN stream(9.3 Million) <-- Results : (Unidirectional, on DECAP side) 6.53 Million(Before Patch) 8.17 Million(After Patch) Bidirectional 4.14 Million (Before patch, on each side) 4.43 Million(After the patch, on each side) > > * I think we need to have a clearer way to pass around checksum state rather > than as an argument, it's just too invasive. A natural place that most OSs use > is in metadata associate with the packet itself, which would be struct > dp_packet in this case. It's also important to consider how this will be > expanded over time. For example, offloading of transmit checksums would > likely want to use the same fields. > > * Please find a way to do this without duplicating the tunnel header > extraction code. Having two copies will surely result in bugs over time. > Potentially the previous point may help with this. > > * Please try to generalize your code where possible. In this case it looks > like > IPv6, Geneve, and GRE (at least the IP checksum) could easily be supported > using the same mechanism. [Sugesh] Modified the code to use the packet metadata and same header extraction functions. However we notice 1-2% of less in performance improvement due to the overhead of packet metadata handling and validations in the extraction code. And also in both case we noticed a~1% of performance drop in PHY-PHY bidirectional test. This is because of the new invalid checksum flag check on every packet from the NIC. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
