On Thu, Aug 20, 2015 at 04:52:29PM +0530, Santosh Shukla wrote:
> On 20 August 2015 at 16:23, Jerin Jacob <jerin.ja...@caviumnetworks.com>
> wrote:
> > On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
> >> On 20 August 2015 at 14:48, Balasubramanian Manoharan
> >> <bala.manoha...@linaro.org> wrote:
> >> >
> >> >
> >> > On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
> >> >>
> >> >> On 15 August 2015 at 00:12, Zoltan Kiss <zoltan.k...@linaro.org> wrote:
> >> >>>
> >> >>> Applications can read the computed hash (if any) and set it if they
> >> >>> changed the packet headers or if the platform haven't calculated the
> >> >>> hash.
> >> >>>
> >> >>> Signed-off-by: Zoltan Kiss <zoltan.k...@linaro.org>
> >> >>> ---
> >> >>> v2:
> >> >>> - focus on RSS hash only
> >> >>> - use setter/getter's
> >> >>>
> >> >>> v3:
> >> >>> - do not mention pointers
> >> >>> - add a note
> >> >>> - add new patches for implementation and test
> >> >>>
> >> >>> include/odp/api/packet.h | 30 ++++++++++++++++++++++++++++++
> >> >>> 1 file changed, 30 insertions(+)
> >> >>>
> >> >>> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> >> >>> index 3a454b5..1ae24bc 100644
> >> >>> --- a/include/odp/api/packet.h
> >> >>> +++ b/include/odp/api/packet.h
> >> >>> @@ -48,6 +48,11 @@ extern "C" {
> >> >>> * Invalid packet segment
> >> >>> */
> >> >>>
> >> >>> +/**
> >> >>> + * @def ODP_PACKET_RSS_INVALID
> >> >>> + * RSS hash is not set
> >> >>> + */
> >> >>> +
> >> >>> /*
> >> >>> *
> >> >>> * Alloc and free
> >> >>> @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
> >> >>> int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
> >> >>>
> >> >>> /**
> >> >>> + * RSS hash value
> >> >>> + *
> >> >>> + * Returns the RSS hash stored for the packet.
> >> >>> + *
> >> >>> + * @param pkt Packet handle
> >> >>> + *
> >> >>> + * @return Hash value
> >> >>> + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
> >> >>> + */
> >> >>> +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
> >> >>> +
> >> >>> +/**
> >> >>> + * Set RSS hash value
> >> >>> + *
> >> >>> + * Store the RSS hash for the packet.
> >> >>> + *
> >> >>> + * @param pkt Packet handle
> >> >>> + * @param rss_hash Hash value to set
> >> >>> + *
> >> >>> + * @note If the application changes the packet header, it might want
> >> >>> to
> >> >>> + * recalculate this value and set it.
> >> >>> + */
> >> >>> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);
> >> >
> >> > Can this use-case be handled by calling
> >> > odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss
> >> > gets
> >> > generated by the implementation rather than being set from application
> >> > using
> >> > odp_packet_set_rss_hash() function?
> >> >
> >>
> >> Bala, As we discussed and in summary for rest; Considering ovs example
> >> : ovs uses sw based rss_hash only when HW not supporting rss Or HW
> >> generated rss is zero.
> >>
> >> hash = packet_get_hash(packet);
> >> if (OVS_UNLIKELY(!hash)) {
> >> hash = miniflow_hash_5tuple(mf, 0);
> >> packet_set_hash(packet, hash);
> >> }
> >> return hash;
> >>
> >> and rss_hash generation is inside ovs dp_packet (middle layer), It is
> >
> > How about following change in OVS plarform specific packet_get_hash code.
> > odp_packet_rss_hash_set
> > kind of interface wont fit correctly in octeon platform as hash
> > identifier used by hardware in subsequent HW based queue operations
> >
> > static inline uint32_t
> > packet_get_hash(struct dp_packet *p)
> > {
> > #ifdef DPDK_NETDEV
> > return p->mbuf.hash.rss;
> > #else
> > #ifdef ODP_NETDEV
> > unit32_t hash;
> > hash = odp_packet_rss_hash(p->odp_pkt);
> > if (OVS_UNLIKELY(!hash)
> > hash = odp_packet_generate_rss_hash(p->odp_pkt);
> > return hash;
> > #endif
> > return p->rss_hash;
> > #endif
> > }
> >
>
> Trying to understand your api definition;
> odp_packet_rss_hash() -> to get rss
> and
> odp_packet_generate_rss_hash() -> generate rss from where : sw or hw?
NOP if HW is capable, for software generated packet/ non-capable HW it
will be in SW
> In either case, your trying to generate non-zero rss (considering a
> valid rss (?) for your platform).
IMO, The term RSS may not be correct in API definition. May be something
like flow id, make sense across all platform.
Jerin
>
> - IIUC _generate_rss_hash() able to take corrective measure so you
> won't find need for using rss_hash_set() right?
> - In other word, No s/w based rss __setting__ required for octeon.
>
> did I understood all that right?
>
> >
> >> with in ovs implementation, Not exposed to application layer, so
> >> application won't need to generate rss / update, so no possibility of
> >> rss mis-match /corruption.
> >>
> >> > Regards,
> >> > Bala
> >> >
> >> >>> +
> >> >>> +/**
> >> >>> * Tests if packet is segmented
> >> >>> *
> >> >>> * @param pkt Packet handle
> >> >>> --
> >> >>
> >> >> Reviewed-by : Santosh Shukla <santosh.shu...@linaro.org>
> >> >>
> >> >>> 1.9.1
> >> >>>
> >> >>> _______________________________________________
> >> >>> lng-odp mailing list
> >> >>> lng-odp@lists.linaro.org
> >> >>> https://lists.linaro.org/mailman/listinfo/lng-odp
> >> >>
> >> >> _______________________________________________
> >> >> lng-odp mailing list
> >> >> lng-odp@lists.linaro.org
> >> >> https://lists.linaro.org/mailman/listinfo/lng-odp
> >> >
> >> >
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
