On 15/06/2015 12:16, "Traynor, Kevin" <kevin.tray...@intel.com> wrote:
> >> -----Original Message----- >> From: Traynor, Kevin >> Sent: Thursday, June 11, 2015 4:55 PM >> To: Daniele Di Proietto; Flavio Leitner >> Cc: dev@openvswitch.org; Gray, Mark D >> Subject: RE: [ovs-dev] Is this an issue for DPDK vhost rss? >> >> >> > -----Original Message----- >> > From: Daniele Di Proietto [mailto:diproiet...@vmware.com] >> > Sent: Thursday, June 11, 2015 4:36 PM >> > To: Flavio Leitner >> > Cc: dev@openvswitch.org; Traynor, Kevin; Gray, Mark D >> > Subject: Re: [ovs-dev] Is this an issue for DPDK vhost rss? >> > >> > >> > >> > On 11/06/2015 15:23, "Flavio Leitner" <f...@sysclose.org> wrote: >> > >> > >On Thu, Jun 11, 2015 at 12:45:10PM +0000, Daniele Di Proietto wrote: >> > >> >> > >> >> > >> On 11/06/2015 04:25, "Flavio Leitner" <f...@sysclose.org> wrote: >> > >> >> > >> >On Wed, Jun 10, 2015 at 03:13:21PM +0000, Daniele Di Proietto >>wrote: >> > >> >> >> > >> >> >> > >> >> On 10/06/2015 12:48, "Gray, Mark D" <mark.d.g...@intel.com> >>wrote: >> > >> >> > >> > >> >> >The vhost port won't generate an RSS hash because it is a >>virtual >> > >>NIC. >> > >> >> > >> > >> >> >> >> > >> >> > >> > >> >> >> It doesn't cause a problem, just make the pkt fall into a >>slow >> > >>path, >> > >> >> >>should we >> > >> >> > >> > >> >> >> fix it? >> > >> >> >> > >> >> Thanks for investigating this. We should definitely fix it. >> > >> >> >> > >> >> > >> > >> >> >> The flag ol_flags may be useful for OVS or let DPDK fix this >>in >> > >>vhost >> > >> >> >>rcv. >> > >> >> > >> > >> >> > >> > >> >> > >> > >> >> >How do you propose that this would work? The RSS would still >>have >> > >>to be >> > >> >> > >> > >> >> >generated in software. >> > >> >> > >> > >> >> >> >> > >> >> >> > >> >> A simple solution would be to reset the RSS hash inside >> > >> >> netdev_dpdk_vhost_rxq_recv(). Other netdev providers that do >>not >> > >> >>support >> > >> >> reading the RSS hash (netdev-linux, netdev-bsd) call >> > >> >> dp_packet_set_rss_hash(pkt, 0) on every received packet. >> > >> >> >> > >> >> This would probably have a small impact on performance, but it's >> > >>better >> > >> >> than trashing the exact match cache. >> > >> >> >> > >> >> I don't believe there's anything that DPDK can do here (like >> > >>resetting >> > >> >>the >> > >> >> hash when the packet is freed), but please correct me if I'm >>wrong. >> > >> >> >> > >> >> Thoughts? >> > >> > >> > >> >I think Dongjun is right and we should use ol_flags. >> > >> >Something like this: >> > >> > >> > >> >diff --git a/lib/dp-packet.h b/lib/dp-packet.h >> > >> >index e4c2593..ad315d1 100644 >> > >> >--- a/lib/dp-packet.h >> > >> >+++ b/lib/dp-packet.h >> > >> >@@ -518,7 +518,12 @@ static inline uint32_t >> > >> > dp_packet_get_rss_hash(struct dp_packet *p) >> > >> > { >> > >> > #ifdef DPDK_NETDEV >> > >> >- return p->mbuf.hash.rss; >> > >> >+ if (p->mbuf.ol_flags & PKT_RX_RSS_HASH) { >> > >> >+ return p->mbuf.hash.rss; >> > >> >+ } >> > >> >+ else { >> > >> >+ return 0; >> > >> >+ } >> > >> > #else >> > >> > return p->rss_hash; >> > >> > #endif >> > >> > >> > >> > >> > >> >I haven't tested that but rte_vhost_dequeue_burst() does >> > >> >ol_flags = 0, so this would be a generic solution for all >> > >> >dpdk devices. >> > >> > >> > >> >Thoughts? >> > >> > >> > >> >fbl >> > >> >> > >> This seems a much better solution. I had no idea that ol_flags >> > >> included PKT_RX_RSS_HASH. >> > >> >> > >> Unfortunately, I tested it and it appears that >>ixgbe_recv_pkts_vec() >> > >> is not setting the flag correctly (it is documented in the >>comments). >> > >> Other ideas? >> > > >> > >The ixgbe_recv_pkts_vec() should at least set ol_flags to zero >> > >because it isn't providing a valid hash and what you get is garbage >> > >from the OVS stack. Is that what you are seeing? >> > >> > No, the problem is the opposite: ixgbe_recv_pkts_vec() returns a valid >> > rss hash, but doesn't set PKT_RX_RSS_HASH in ol_flags. >> >> agreed, that's what I'm seeing. >> >> This doesn't >> > affect correctness, but it forces us to always calculate the hash in >> > software. >> >> not with the present code, but with the fix suggested by Flavio it >>would. >> >> > >> > Is it possible to make ixgbe_recv_pkts_vec() set PKT_RX_RSS_HASH in >>DPDK? >> >> we'll find out about this - not sure if there's a reason for not >>setting it. > >There is a dpdk patchset that contains a potential fix for this and lots >of >other changes, but I haven't tested yet. >https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_ml_archives_d >ev_2015-2DJune_018436.html&d=BQIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMN >tXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=FDVPKa2SqwpyYOTmA2 >zGdscCPa1FVdQG3Zbr4tHrp38&s=fjg7wArWvYLJlgEGKijK6W6ECAxGk660UrPF3rAr4Rs&e= > > >> >> > Otherwise, should we avoid using the vectorized version? >> >> that's debatable - from a performance view it may be better to leave it >>in >> and take the hit elsewhere for the time being if there's a possibility >>that >> it will be changed in DPDK later. > >With a loop to reset the rss after the rte_vhost_dequeue_burst() call I'm >seeing a drop of ~100kpps in vhost performance. Rx vectoristion gives a >gain >of about ~1 mpps on my system for the phy2phy cases. > >Using the ol_flags check is the right option when DPDK supports setting it >correctly with rx vectorisation. In the meantime there's choice of using >the >reset loop or removing rx vectorisation - what do you think? Thanks for sharing these results. I've observed that if OVS can't use the RSS hash and has to compute we lose ~2Mpps on a single flow phy2phy test. Despite this, I still think we should consider the ol_flags because: * DPDK drivers (other than ixgbe) should use ol_flags as well to mark the RSS hash as valid * ixgbe_recv_pkts_vec() will report PKT_RX_RSS_HASH in future releases (the patch you sent will be effective since DPDK 2.2, right?) If the throughput with the non-vector rx routine is higher we can disable the vector rx as a temporary workaround. I'll do some tests and send a patch tomorrow. Daniele > >> >> > >> > That said, we probably should go with your solution anyway. >> > >> > >If so, ixgbe_recv_pkts_vec() needs to set ol_flags to zero. This will >> > >fix for every user of dpdk, not just OVS. >> > > >> > >As a possible workaround for OVS we could fix >>dp_netdev_process_rxq_port() >> > >to memset(0) the packets mbufs array which should be faster than loop >> > >through the array and set fields to zero. >> > >> > I don't understand what you mean here exactly >> > >> > Thanks, >> > >> > Daniele > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
