> -----Original Message-----
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
> Proietto
> Sent: Tuesday, June 16, 2015 7:39 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
>
> DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is
> set in 'ol_flags'. Otherwise the hash is garbage and doesn't
> relate to the packet.
>
> This fixes an issue with vhost, which, being a virtual NIC, doesn't
> compute the hash.
>
> Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing
> OVS to compute an hash is software. This has a significant impact on
> performance (-30% throughput in a single flow setup) which can be
> mitigated in the CPU supports crc32c instructions.
As per the other thread on this I'm a bit concerned about the performance
drop from this patch, so I did some testing of this and alternative/
complimentary solutions.
Here's the options I looked at and some comments:
1. This patch in isolation: vhost drops about ~15% vhost-vhost and
phy-vhost-phy (because of sw hash) but also there is drops of ~25% for
phy-phy and ~15% drop for phy-ivshmem-phy.
2. Leave the code as is and let EMC misses happen for vhost rx pkts:
I measure this at ~35% drop if missed *everytime* for vhost-vhost. We
see in testing that it can also never happen, but this is not realistic.
There should be no impact to other DPDK interfaces.
3. Add hash reset for packets from vhost: This is another way of forcing
the software hash for vhost rx and it is roughly equivalent in performance
to 1. for vhost-vhost (~15% drop). While there is a no significant drop
for phy-vhost-phy. There should be no impact to other DPDK interfaces.
4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop
~15% as per 1. and there should be nothing significant for phy-vhost-phy.
We would lose the 10% gain that rx vectorisation gave us for phy-phy.
There should be no impact for dpdkr ports.
In terms of not knowing whether the hw hash is valid or not if the flag is
not checked, I would have expected the pmd to return an error on config if
the hash wasn't supported, but I'm not sure that it does.
In the worst case where there was an incorrect hash, it would miss the EMC
which is about a 45% drop for phy-phy. I would think it's pretty safe that
if we configure it, the hash will be correct but I guess there is a
possibility it wouldn't be.
Even if it is possible to get a smaller patch to fix the underlying issue
in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance
would remain low until sometime in August. If it's DPDK 2.2, then it would
be sometime in December. This would mean any performance drops would be
present in OVS 2.4 and possibly OVS 2.5.
Sorry :( but based on the performance drop with this patch in isolation it
would be a NAK from me. My preference would be 3 which gives best performance,
or 4 which is a bit lower for phy-phy but safer.
Kevin.
>
> Reported-by: Dongjun <do...@dtdream.com>
> Suggested-by: Flavio Leitner <f...@sysclose.org>
> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
> ---
> lib/dp-packet.h | 11 +++++++++++
> lib/dpif-netdev.c | 2 +-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index e4c2593..6840750 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -529,11 +529,22 @@ dp_packet_set_rss_hash(struct dp_packet *p, uint32_t
> hash)
> {
> #ifdef DPDK_NETDEV
> p->mbuf.hash.rss = hash;
> + p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> #else
> p->rss_hash = hash;
> #endif
> }
>
> +static inline bool
> +dp_packet_rss_valid(struct dp_packet *p)
> +{
> +#ifdef DPDK_NETDEV
> + return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> +#else
> + return true;
> +#endif
> +}
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index f13169c..c4a4b3a 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3036,7 +3036,7 @@ dpif_netdev_packet_get_rss_hash(struct dp_packet
> *packet,
> {
> uint32_t hash, recirc_depth;
>
> - hash = dp_packet_get_rss_hash(packet);
> + hash = dp_packet_rss_valid(packet) ? dp_packet_get_rss_hash(packet) : 0;
> if (OVS_UNLIKELY(!hash)) {
> hash = miniflow_hash_5tuple(mf, 0);
> dp_packet_set_rss_hash(packet, hash);
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
