| Message ID | 1441813536-14086-1-git-send-email-diproiettod@vmware.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
> -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Daniele Di > Proietto > Sent: Wednesday, September 9, 2015 4:46 PM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH v2] 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. > > Reported-by: Dongjun <dongj@dtdream.com> > Suggested-by: Flavio Leitner <fbl@sysclose.org> > Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> > --- > v1 -> v2: > > * Added a comment above dp_packet_get_rss_hash() > * Added an OVS_UNUSED attribute on dp_packet_rss_valid() > --- Thanks Daniele - I tested and working as expected. Acked-by: Kevin Traynor <kevin.traynor@intel.com> > lib/dp-packet.h | 13 +++++++++++++ > lib/dpif-netdev.c | 2 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index e4c2593..5532bee 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -514,6 +514,8 @@ dp_packet_reset_packet(struct dp_packet *b, int off) > b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX; > } > > +/* Returns the RSS hash of the packet 'p'. Note that the returned value is > + * correct only if 'dp_packet_rss_valid(p)' returns true */ > static inline uint32_t > dp_packet_get_rss_hash(struct dp_packet *p) > { > @@ -529,11 +531,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 OVS_UNUSED) > +{ > +#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 db76290..490ced3 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3100,7 +3100,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
On Wed, Sep 9, 2015 at 8:45 AM, Daniele Di Proietto <diproiettod@vmware.com> 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. > > Reported-by: Dongjun <dongj@dtdream.com> > Suggested-by: Flavio Leitner <fbl@sysclose.org> > Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> > --- > v1 -> v2: > > * Added a comment above dp_packet_get_rss_hash() > * Added an OVS_UNUSED attribute on dp_packet_rss_valid() > --- > lib/dp-packet.h | 13 +++++++++++++ > lib/dpif-netdev.c | 2 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index e4c2593..5532bee 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -514,6 +514,8 @@ dp_packet_reset_packet(struct dp_packet *b, int off) > b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX; > } > > +/* Returns the RSS hash of the packet 'p'. Note that the returned value is > + * correct only if 'dp_packet_rss_valid(p)' returns true */ > static inline uint32_t > dp_packet_get_rss_hash(struct dp_packet *p) > { > @@ -529,11 +531,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 OVS_UNUSED) > +{ > +#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 db76290..490ced3 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3100,7 +3100,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)) { With new valid hash bit, zero hash should be a valid hash value. > 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
On 09/09/2015 18:56, "Pravin Shelar" <pshelar@nicira.com> wrote: >On Wed, Sep 9, 2015 at 8:45 AM, Daniele Di Proietto ><diproiettod@vmware.com> 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. >> >> Reported-by: Dongjun <dongj@dtdream.com> >> Suggested-by: Flavio Leitner <fbl@sysclose.org> >> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> >> --- >> v1 -> v2: >> >> * Added a comment above dp_packet_get_rss_hash() >> * Added an OVS_UNUSED attribute on dp_packet_rss_valid() >> --- >> lib/dp-packet.h | 13 +++++++++++++ >> lib/dpif-netdev.c | 2 +- >> 2 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/lib/dp-packet.h b/lib/dp-packet.h >> index e4c2593..5532bee 100644 >> --- a/lib/dp-packet.h >> +++ b/lib/dp-packet.h >> @@ -514,6 +514,8 @@ dp_packet_reset_packet(struct dp_packet *b, int off) >> b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX; >> } >> >> +/* Returns the RSS hash of the packet 'p'. Note that the returned >>value is >> + * correct only if 'dp_packet_rss_valid(p)' returns true */ >> static inline uint32_t >> dp_packet_get_rss_hash(struct dp_packet *p) >> { >> @@ -529,11 +531,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 OVS_UNUSED) >> +{ >> +#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 db76290..490ced3 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -3100,7 +3100,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)) { >With new valid hash bit, zero hash should be a valid hash value. Sorry for missing the comment on the first round of review. That makes sense, thanks. I've had to update the netdev-*, but nothing huge. I've added Kevin's ack (thanks!) and sent a v3 to the list. > >> hash = miniflow_hash_5tuple(mf, 0); >> dp_packet_set_rss_hash(packet, hash); >> -- >> 2.1.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=aiBMpWGcN8uLcOKO0aWc5niy8k3 >>f-_mg9ESXpnz6svk&s=dLaxeoV5Y1E1LzfKn8h1a8Ge_gMMDuOv-dWkCC5YtW4&e=
diff --git a/lib/dp-packet.h b/lib/dp-packet.h index e4c2593..5532bee 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -514,6 +514,8 @@ dp_packet_reset_packet(struct dp_packet *b, int off) b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX; } +/* Returns the RSS hash of the packet 'p'. Note that the returned value is + * correct only if 'dp_packet_rss_valid(p)' returns true */ static inline uint32_t dp_packet_get_rss_hash(struct dp_packet *p) { @@ -529,11 +531,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 OVS_UNUSED) +{ +#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 db76290..490ced3 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3100,7 +3100,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);
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. Reported-by: Dongjun <dongj@dtdream.com> Suggested-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> --- v1 -> v2: * Added a comment above dp_packet_get_rss_hash() * Added an OVS_UNUSED attribute on dp_packet_rss_valid() --- lib/dp-packet.h | 13 +++++++++++++ lib/dpif-netdev.c | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-)