On Tue, Jun 16, 2015 at 07:39:00PM +0100, Daniele Di Proietto wrote:
> 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.
>
> 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
Callers of dp_packet_get_rss_hash() have no idea about the ol_flags,
so this could lead to other problems in the future. But the
alternative would be to hash at dp_packet_get_rss_hash() which
doesn't have parsed data in miniflow/flow.
So, could you please add a comment on top of dp_packet_get_rss_hash()
about dp_packet_rss_valid()?
Otherwise the patch looks good to me.
Thanks!
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
