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.  This doesn't
affect correctness, but it forces us to always calculate the hash in
software.

Is it possible to make ixgbe_recv_pkts_vec() set PKT_RX_RSS_HASH in DPDK?
Otherwise, should we avoid using the vectorized version?

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

Reply via email to