Hi,
On 20 August 2015 at 17:32, Bill Fischofer <bill.fischo...@linaro.org>
wrote:
> The RSS is relevant to packets originating from a NIC and is independent
> of the CoS or other flow designators. It's there mainly because some
> applications (e.g., OVS) use it internally, so it's for legacy support.
>
This API might be used for legacy applications but if the HW generates RSS
by default then that value can be used by the application, somehow this API
is seen as a requirement only for OVS but getting the hash value of the
packet will be used by other applications also. So we should solve the
generic use-case rather than focussing for OVS alone.
The issue here is to avoid the application to configure the hash value
since it is possible that the application configures a hash value different
from what the implementation would have generated for that particular
packet flow and this would cause an issue in some platforms. This was the
basis on which the removal of set_rss_hash() function was suggested.
Regards,
Bala
>
> On Thu, Aug 20, 2015 at 6:43 AM, Jerin Jacob <
> jerin.ja...@caviumnetworks.com> wrote:
>
>> 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
>>
>
>
> _______________________________________________
> 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
