diff mbox

[API-NEXT,v3,2/3] linux-generic: packet: implement RSS hash support

Message ID 1439577746-21499-2-git-send-email-zoltan.kiss@linaro.org
State New
Headers show

Commit Message

Zoltan Kiss Aug. 14, 2015, 6:42 p.m. UTC
This platform doesn't compute it, packet_init set it to 0, which happens
to be ODP_PACKET_RSS_INVALID.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---
 platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
 platform/linux-generic/include/odp_packet_internal.h   |  1 +
 platform/linux-generic/odp_packet.c                    | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

Comments

Maxim Uvarov Aug. 17, 2015, 6:24 a.m. UTC | #1
On 08/14/15 21:42, Zoltan Kiss wrote:
> This platform doesn't compute it, packet_init set it to 0, which happens
> to be ODP_PACKET_RSS_INVALID.
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> ---
>   platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
>   platform/linux-generic/include/odp_packet_internal.h   |  1 +
>   platform/linux-generic/odp_packet.c                    | 14 ++++++++++++++
>   3 files changed, 17 insertions(+)
>
> diff --git a/platform/linux-generic/include/odp/plat/packet_types.h b/platform/linux-generic/include/odp/plat/packet_types.h
> index 45cb801..afc72f4 100644
> --- a/platform/linux-generic/include/odp/plat/packet_types.h
> +++ b/platform/linux-generic/include/odp/plat/packet_types.h
> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_packet_seg_t);
>   
>   #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t, 0xffffffff)
>   
> +#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)
> +
>   /** Get printable format of odp_packet_t */
>   static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
>   {
> diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h
> index 8c41491..6e52713 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -136,6 +136,7 @@ typedef struct {
>   	uint32_t tailroom;
>   
>   	odp_pktio_t input;
> +	uint32_t rss_hash;      /**< RSS hash value*/

is rss_hash always and everywhere 32 bit?

Maxim.


>   
>   	odp_crypto_generic_op_result_t op_result;  /**< Result for crypto */
>   } odp_packet_hdr_t;
> diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
> index 5581cc4..649071f 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -326,6 +326,20 @@ int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset)
>   	return 0;
>   }
>   
> +uint32_t odp_packet_rss_hash(odp_packet_t pkt)
> +{
> +	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
> +
> +	return pkt_hdr->rss_hash;
> +}
> +
> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
> +{
> +	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
> +
> +	pkt_hdr->rss_hash = rss_hash;
> +}
> +
>   int odp_packet_is_segmented(odp_packet_t pkt)
>   {
>   	return odp_packet_hdr(pkt)->buf_hdr.segcount > 1;
Santosh Shukla Aug. 17, 2015, 6:29 a.m. UTC | #2
On 17 August 2015 at 11:54, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 08/14/15 21:42, Zoltan Kiss wrote:
>>
>> This platform doesn't compute it, packet_init set it to 0, which happens
>> to be ODP_PACKET_RSS_INVALID.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>>   platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
>>   platform/linux-generic/include/odp_packet_internal.h   |  1 +
>>   platform/linux-generic/odp_packet.c                    | 14
>> ++++++++++++++
>>   3 files changed, 17 insertions(+)
>>
>> diff --git a/platform/linux-generic/include/odp/plat/packet_types.h
>> b/platform/linux-generic/include/odp/plat/packet_types.h
>> index 45cb801..afc72f4 100644
>> --- a/platform/linux-generic/include/odp/plat/packet_types.h
>> +++ b/platform/linux-generic/include/odp/plat/packet_types.h
>> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_packet_seg_t);
>>     #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t,
>> 0xffffffff)
>>   +#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)
>> +
>>   /** Get printable format of odp_packet_t */
>>   static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
>>   {
>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
>> b/platform/linux-generic/include/odp_packet_internal.h
>> index 8c41491..6e52713 100644
>> --- a/platform/linux-generic/include/odp_packet_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_internal.h
>> @@ -136,6 +136,7 @@ typedef struct {
>>         uint32_t tailroom;
>>         odp_pktio_t input;
>> +       uint32_t rss_hash;      /**< RSS hash value*/
>
>
> is rss_hash always and everywhere 32 bit?
>

It is nic specific; If you checkout latest nics; rss_hash is 32bit
word long. so Yes.

My Reviewed-by to this patch.

> Maxim.
>
>
>>         odp_crypto_generic_op_result_t op_result;  /**< Result for crypto
>> */
>>   } odp_packet_hdr_t;
>> diff --git a/platform/linux-generic/odp_packet.c
>> b/platform/linux-generic/odp_packet.c
>> index 5581cc4..649071f 100644
>> --- a/platform/linux-generic/odp_packet.c
>> +++ b/platform/linux-generic/odp_packet.c
>> @@ -326,6 +326,20 @@ int odp_packet_l4_offset_set(odp_packet_t pkt,
>> uint32_t offset)
>>         return 0;
>>   }
>>   +uint32_t odp_packet_rss_hash(odp_packet_t pkt)
>> +{
>> +       odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>> +
>> +       return pkt_hdr->rss_hash;
>> +}
>> +
>> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
>> +{
>> +       odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>> +
>> +       pkt_hdr->rss_hash = rss_hash;
>> +}
>> +
>>   int odp_packet_is_segmented(odp_packet_t pkt)
>>   {
>>         return odp_packet_hdr(pkt)->buf_hdr.segcount > 1;
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Zoltan Kiss Aug. 17, 2015, 8:39 a.m. UTC | #3
On 17/08/15 07:24, Maxim Uvarov wrote:
> On 08/14/15 21:42, Zoltan Kiss wrote:
>> This platform doesn't compute it, packet_init set it to 0, which happens
>> to be ODP_PACKET_RSS_INVALID.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>>   platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
>>   platform/linux-generic/include/odp_packet_internal.h   |  1 +
>>   platform/linux-generic/odp_packet.c                    | 14
>> ++++++++++++++
>>   3 files changed, 17 insertions(+)
>>
>> diff --git a/platform/linux-generic/include/odp/plat/packet_types.h
>> b/platform/linux-generic/include/odp/plat/packet_types.h
>> index 45cb801..afc72f4 100644
>> --- a/platform/linux-generic/include/odp/plat/packet_types.h
>> +++ b/platform/linux-generic/include/odp/plat/packet_types.h
>> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_packet_seg_t);
>>   #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t,
>> 0xffffffff)
>> +#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)

This define is incorrect, I think.

>> +
>>   /** Get printable format of odp_packet_t */
>>   static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
>>   {
>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
>> b/platform/linux-generic/include/odp_packet_internal.h
>> index 8c41491..6e52713 100644
>> --- a/platform/linux-generic/include/odp_packet_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_internal.h
>> @@ -136,6 +136,7 @@ typedef struct {
>>       uint32_t tailroom;
>>       odp_pktio_t input;
>> +    uint32_t rss_hash;      /**< RSS hash value*/
>
> is rss_hash always and everywhere 32 bit?

I don't think it's guaranteed anywhere. Maybe we should rather use an 
opaque type? But then we would need to provide operator functions for 
all the possible uses, that would be an overkill.
>
> Maxim.
>
>
>>       odp_crypto_generic_op_result_t op_result;  /**< Result for
>> crypto */
>>   } odp_packet_hdr_t;
>> diff --git a/platform/linux-generic/odp_packet.c
>> b/platform/linux-generic/odp_packet.c
>> index 5581cc4..649071f 100644
>> --- a/platform/linux-generic/odp_packet.c
>> +++ b/platform/linux-generic/odp_packet.c
>> @@ -326,6 +326,20 @@ int odp_packet_l4_offset_set(odp_packet_t pkt,
>> uint32_t offset)
>>       return 0;
>>   }
>> +uint32_t odp_packet_rss_hash(odp_packet_t pkt)
>> +{
>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>> +
>> +    return pkt_hdr->rss_hash;
>> +}
>> +
>> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
>> +{
>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>> +
>> +    pkt_hdr->rss_hash = rss_hash;
>> +}
>> +
>>   int odp_packet_is_segmented(odp_packet_t pkt)
>>   {
>>       return odp_packet_hdr(pkt)->buf_hdr.segcount > 1;
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Santosh Shukla Aug. 17, 2015, 8:45 a.m. UTC | #4
On 17 August 2015 at 14:09, Zoltan Kiss <zoltan.kiss@linaro.org> wrote:
>
>
> On 17/08/15 07:24, Maxim Uvarov wrote:
>>
>> On 08/14/15 21:42, Zoltan Kiss wrote:
>>>
>>> This platform doesn't compute it, packet_init set it to 0, which happens
>>> to be ODP_PACKET_RSS_INVALID.
>>>
>>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>>> ---
>>>   platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
>>>   platform/linux-generic/include/odp_packet_internal.h   |  1 +
>>>   platform/linux-generic/odp_packet.c                    | 14
>>> ++++++++++++++
>>>   3 files changed, 17 insertions(+)
>>>
>>> diff --git a/platform/linux-generic/include/odp/plat/packet_types.h
>>> b/platform/linux-generic/include/odp/plat/packet_types.h
>>> index 45cb801..afc72f4 100644
>>> --- a/platform/linux-generic/include/odp/plat/packet_types.h
>>> +++ b/platform/linux-generic/include/odp/plat/packet_types.h
>>> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_packet_seg_t);
>>>   #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t,
>>> 0xffffffff)
>>> +#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)
>
>
> This define is incorrect, I think.
>
>>> +
>>>   /** Get printable format of odp_packet_t */
>>>   static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
>>>   {
>>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
>>> b/platform/linux-generic/include/odp_packet_internal.h
>>> index 8c41491..6e52713 100644
>>> --- a/platform/linux-generic/include/odp_packet_internal.h
>>> +++ b/platform/linux-generic/include/odp_packet_internal.h
>>> @@ -136,6 +136,7 @@ typedef struct {
>>>       uint32_t tailroom;
>>>       odp_pktio_t input;
>>> +    uint32_t rss_hash;      /**< RSS hash value*/
>>
>>
>> is rss_hash always and everywhere 32 bit?
>
>
> I don't think it's guaranteed anywhere. Maybe we should rather use an opaque
> type? But then we would need to provide operator functions for all the
> possible uses, that would be an overkill.
>

So far per my knowledge; Plenty of NIC's example e100/ixgbe/brcm etc
provide 32bit word size. You could add opaque support, but make sure
handle/method are inlined (less expensive); Because its per-pkt
Otherwise overkill. I would prefer keeping it 32bit word size unless
someone speaks / describe their experience on using rss_hash 64bit
word size.

>>
>> Maxim.
>>
>>
>>>       odp_crypto_generic_op_result_t op_result;  /**< Result for
>>> crypto */
>>>   } odp_packet_hdr_t;
>>> diff --git a/platform/linux-generic/odp_packet.c
>>> b/platform/linux-generic/odp_packet.c
>>> index 5581cc4..649071f 100644
>>> --- a/platform/linux-generic/odp_packet.c
>>> +++ b/platform/linux-generic/odp_packet.c
>>> @@ -326,6 +326,20 @@ int odp_packet_l4_offset_set(odp_packet_t pkt,
>>> uint32_t offset)
>>>       return 0;
>>>   }
>>> +uint32_t odp_packet_rss_hash(odp_packet_t pkt)
>>> +{
>>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>>> +
>>> +    return pkt_hdr->rss_hash;
>>> +}
>>> +
>>> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
>>> +{
>>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
>>> +
>>> +    pkt_hdr->rss_hash = rss_hash;
>>> +}
>>> +
>>>   int odp_packet_is_segmented(odp_packet_t pkt)
>>>   {
>>>       return odp_packet_hdr(pkt)->buf_hdr.segcount > 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
Bill Fischofer Aug. 19, 2015, 8:45 p.m. UTC | #5
The RSS Hash is the output of the Toepliz Hash function.  It is used by
masking some number of low-order bits of this function to get the RSS Index
used.  As such 32-bits is more than sufficient since it is essentially maps
to the number of processing cores.  A 32-bit hash supports 4 billion cores,
well beyond today's largest systems.

I'd leave it as uint32_t for now.

On Mon, Aug 17, 2015 at 3:45 AM, Santosh Shukla <santosh.shukla@linaro.org>
wrote:

> On 17 August 2015 at 14:09, Zoltan Kiss <zoltan.kiss@linaro.org> wrote:
> >
> >
> > On 17/08/15 07:24, Maxim Uvarov wrote:
> >>
> >> On 08/14/15 21:42, Zoltan Kiss wrote:
> >>>
> >>> This platform doesn't compute it, packet_init set it to 0, which
> happens
> >>> to be ODP_PACKET_RSS_INVALID.
> >>>
> >>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> >>> ---
> >>>   platform/linux-generic/include/odp/plat/packet_types.h |  2 ++
> >>>   platform/linux-generic/include/odp_packet_internal.h   |  1 +
> >>>   platform/linux-generic/odp_packet.c                    | 14
> >>> ++++++++++++++
> >>>   3 files changed, 17 insertions(+)
> >>>
> >>> diff --git a/platform/linux-generic/include/odp/plat/packet_types.h
> >>> b/platform/linux-generic/include/odp/plat/packet_types.h
> >>> index 45cb801..afc72f4 100644
> >>> --- a/platform/linux-generic/include/odp/plat/packet_types.h
> >>> +++ b/platform/linux-generic/include/odp/plat/packet_types.h
> >>> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_packet_seg_t);
> >>>   #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t,
> >>> 0xffffffff)
> >>> +#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)
> >
> >
> > This define is incorrect, I think.
> >
> >>> +
> >>>   /** Get printable format of odp_packet_t */
> >>>   static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
> >>>   {
> >>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
> >>> b/platform/linux-generic/include/odp_packet_internal.h
> >>> index 8c41491..6e52713 100644
> >>> --- a/platform/linux-generic/include/odp_packet_internal.h
> >>> +++ b/platform/linux-generic/include/odp_packet_internal.h
> >>> @@ -136,6 +136,7 @@ typedef struct {
> >>>       uint32_t tailroom;
> >>>       odp_pktio_t input;
> >>> +    uint32_t rss_hash;      /**< RSS hash value*/
> >>
> >>
> >> is rss_hash always and everywhere 32 bit?
> >
> >
> > I don't think it's guaranteed anywhere. Maybe we should rather use an
> opaque
> > type? But then we would need to provide operator functions for all the
> > possible uses, that would be an overkill.
> >
>
> So far per my knowledge; Plenty of NIC's example e100/ixgbe/brcm etc
> provide 32bit word size. You could add opaque support, but make sure
> handle/method are inlined (less expensive); Because its per-pkt
> Otherwise overkill. I would prefer keeping it 32bit word size unless
> someone speaks / describe their experience on using rss_hash 64bit
> word size.
>
> >>
> >> Maxim.
> >>
> >>
> >>>       odp_crypto_generic_op_result_t op_result;  /**< Result for
> >>> crypto */
> >>>   } odp_packet_hdr_t;
> >>> diff --git a/platform/linux-generic/odp_packet.c
> >>> b/platform/linux-generic/odp_packet.c
> >>> index 5581cc4..649071f 100644
> >>> --- a/platform/linux-generic/odp_packet.c
> >>> +++ b/platform/linux-generic/odp_packet.c
> >>> @@ -326,6 +326,20 @@ int odp_packet_l4_offset_set(odp_packet_t pkt,
> >>> uint32_t offset)
> >>>       return 0;
> >>>   }
> >>> +uint32_t odp_packet_rss_hash(odp_packet_t pkt)
> >>> +{
> >>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
> >>> +
> >>> +    return pkt_hdr->rss_hash;
> >>> +}
> >>> +
> >>> +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
> >>> +{
> >>> +    odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
> >>> +
> >>> +    pkt_hdr->rss_hash = rss_hash;
> >>> +}
> >>> +
> >>>   int odp_packet_is_segmented(odp_packet_t pkt)
> >>>   {
> >>>       return odp_packet_hdr(pkt)->buf_hdr.segcount > 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
>
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp/plat/packet_types.h b/platform/linux-generic/include/odp/plat/packet_types.h
index 45cb801..afc72f4 100644
--- a/platform/linux-generic/include/odp/plat/packet_types.h
+++ b/platform/linux-generic/include/odp/plat/packet_types.h
@@ -36,6 +36,8 @@  typedef ODP_HANDLE_T(odp_packet_seg_t);
 
 #define ODP_PACKET_SEG_INVALID _odp_cast_scalar(odp_packet_seg_t, 0xffffffff)
 
+#define ODP_PACKET_RSS_INVALID _odp_cast_scalar(odp_packet_t, 0)
+
 /** Get printable format of odp_packet_t */
 static inline uint64_t odp_packet_to_u64(odp_packet_t hdl)
 {
diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h
index 8c41491..6e52713 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -136,6 +136,7 @@  typedef struct {
 	uint32_t tailroom;
 
 	odp_pktio_t input;
+	uint32_t rss_hash;      /**< RSS hash value*/
 
 	odp_crypto_generic_op_result_t op_result;  /**< Result for crypto */
 } odp_packet_hdr_t;
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 5581cc4..649071f 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -326,6 +326,20 @@  int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset)
 	return 0;
 }
 
+uint32_t odp_packet_rss_hash(odp_packet_t pkt)
+{
+	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
+
+	return pkt_hdr->rss_hash;
+}
+
+void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash)
+{
+	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
+
+	pkt_hdr->rss_hash = rss_hash;
+}
+
 int odp_packet_is_segmented(odp_packet_t pkt)
 {
 	return odp_packet_hdr(pkt)->buf_hdr.segcount > 1;