> -----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. http://dpdk.org/ml/archives/dev/2015-June/018436.html > > > 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? > > > > > 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
