[PATCHv5,2/2] api: pktio statistics: define start and stop

Message ID 1445510711-12313-3-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Oct. 22, 2015, 10:45 a.m.
Define pktio stats start/stop functions for case when same
statistic module is used by different pktios, so if it's used
for one of them it cannot be used by other, to allow it for
first it should be disable for second. For instance 2 statistic
modules are shared between 4 eth ports, etc.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 include/odp/api/packet_io_stats.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Savolainen, Petri (Nokia - FI/Espoo) Oct. 23, 2015, 10:35 a.m. | #1
I think these are not generally needed or supported. It's better to add a parameter into odp_pktio_param_t.

For example, 

typedef enum odp_pktio_stats_mode_t {

/** Need basic statistics on this interface */
ODP_PKTIO_STATS_BASIC = 0,
/** Don't need any statistics on this interface */
ODP_PKTIO_STATS_DISABLED
} odp_pktio_stats_mode_t;


-Petri



> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT

> Maxim Uvarov

> Sent: Thursday, October 22, 2015 1:45 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start and

> stop

> 

> Define pktio stats start/stop functions for case when same

> statistic module is used by different pktios, so if it's used

> for one of them it cannot be used by other, to allow it for

> first it should be disable for second. For instance 2 statistic

> modules are shared between 4 eth ports, etc.

> 

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> ---

>  include/odp/api/packet_io_stats.h | 20 ++++++++++++++++++++

>  1 file changed, 20 insertions(+)

> 

> diff --git a/include/odp/api/packet_io_stats.h

> b/include/odp/api/packet_io_stats.h

> index 1bff9ca..03f060e 100644

> --- a/include/odp/api/packet_io_stats.h

> +++ b/include/odp/api/packet_io_stats.h

> @@ -123,6 +123,26 @@ int odp_pktio_stats(odp_pktio_t pktio,

>  int odp_pktio_stats_reset(odp_pktio_t pktio);

> 

>  /**

> + * Start statistics for pktio handle

> + *

> + * @param	pktio	 Packet IO handle

> + * @retval  0 on success

> + * @retval <0 on failure

> + *

> + */

> +int odp_pktio_stats_start(odp_pktio_t pktio);

> +

> +/**

> + * Stop statistics for pktio handle

> + *

> + * @param	pktio	 Packet IO handle

> + * @retval  0 on success

> + * @retval <0 on failure

> + *

> + */

> +int odp_pktio_stats_stop(odp_pktio_t pktio);

> +

> +/**

>   * @}

>   */

> 

> --

> 1.9.1

> 

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp
Ivan Khoronzhuk Oct. 23, 2015, 10:59 a.m. | #2
On 23.10.15 13:35, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
> I think these are not generally needed or supported. It's better to add a parameter into odp_pktio_param_t.

It's needed.
The main reason for that the same statistic module can be used by different
pktios, so if it's used for one of them it cannot be used by other, to allow
it for first it should be disable for second. For instance 2 statistic modules
are shared between 4 eth ports, etc.

>
> For example,
>
> typedef enum odp_pktio_stats_mode_t {
>
> /** Need basic statistics on this interface */
> ODP_PKTIO_STATS_BASIC = 0,
> /** Don't need any statistics on this interface */
> ODP_PKTIO_STATS_DISABLED
> } odp_pktio_stats_mode_t;
>
>
> -Petri
>
>
>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>> Maxim Uvarov
>> Sent: Thursday, October 22, 2015 1:45 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start and
>> stop
>>
>> Define pktio stats start/stop functions for case when same
>> statistic module is used by different pktios, so if it's used
>> for one of them it cannot be used by other, to allow it for
>> first it should be disable for second. For instance 2 statistic
>> modules are shared between 4 eth ports, etc.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   include/odp/api/packet_io_stats.h | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/include/odp/api/packet_io_stats.h
>> b/include/odp/api/packet_io_stats.h
>> index 1bff9ca..03f060e 100644
>> --- a/include/odp/api/packet_io_stats.h
>> +++ b/include/odp/api/packet_io_stats.h
>> @@ -123,6 +123,26 @@ int odp_pktio_stats(odp_pktio_t pktio,
>>   int odp_pktio_stats_reset(odp_pktio_t pktio);
>>
>>   /**
>> + * Start statistics for pktio handle
>> + *
>> + * @param	pktio	 Packet IO handle
>> + * @retval  0 on success
>> + * @retval <0 on failure
>> + *
>> + */
>> +int odp_pktio_stats_start(odp_pktio_t pktio);
>> +
>> +/**
>> + * Stop statistics for pktio handle
>> + *
>> + * @param	pktio	 Packet IO handle
>> + * @retval  0 on success
>> + * @retval <0 on failure
>> + *
>> + */
>> +int odp_pktio_stats_stop(odp_pktio_t pktio);
>> +
>> +/**
>>    * @}
>>    */
>>
>> --
>> 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
>
Savolainen, Petri (Nokia - FI/Espoo) Oct. 23, 2015, 11:03 a.m. | #3
User tells to the implementation on open() which interfaces needed statistics. As long as implementation has enough stat resources open() succeeds. When all stats are gone and user still ask for stats, open() fails. Implementation user manual documents this limitation (how many interfaces can be opened with stats enabled and in which combination).

-Petri


> -----Original Message-----

> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]

> Sent: Friday, October 23, 2015 2:00 PM

> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-

> odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start

> and stop

> 

> 

> 

> On 23.10.15 13:35, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >

> > I think these are not generally needed or supported. It's better to add a

> parameter into odp_pktio_param_t.

> 

> It's needed.

> The main reason for that the same statistic module can be used by different

> pktios, so if it's used for one of them it cannot be used by other, to

> allow

> it for first it should be disable for second. For instance 2 statistic

> modules

> are shared between 4 eth ports, etc.

> 

> >

> > For example,

> >

> > typedef enum odp_pktio_stats_mode_t {

> >

> > /** Need basic statistics on this interface */

> > ODP_PKTIO_STATS_BASIC = 0,

> > /** Don't need any statistics on this interface */

> > ODP_PKTIO_STATS_DISABLED

> > } odp_pktio_stats_mode_t;

> >

> >

> > -Petri

> >

> >

> >

> >> -----Original Message-----

> >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT

> >> Maxim Uvarov

> >> Sent: Thursday, October 22, 2015 1:45 PM

> >> To: lng-odp@lists.linaro.org

> >> Subject: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start and

> >> stop

> >>

> >> Define pktio stats start/stop functions for case when same

> >> statistic module is used by different pktios, so if it's used

> >> for one of them it cannot be used by other, to allow it for

> >> first it should be disable for second. For instance 2 statistic

> >> modules are shared between 4 eth ports, etc.

> >>

> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> >> ---

> >>   include/odp/api/packet_io_stats.h | 20 ++++++++++++++++++++

> >>   1 file changed, 20 insertions(+)

> >>

> >> diff --git a/include/odp/api/packet_io_stats.h

> >> b/include/odp/api/packet_io_stats.h

> >> index 1bff9ca..03f060e 100644

> >> --- a/include/odp/api/packet_io_stats.h

> >> +++ b/include/odp/api/packet_io_stats.h

> >> @@ -123,6 +123,26 @@ int odp_pktio_stats(odp_pktio_t pktio,

> >>   int odp_pktio_stats_reset(odp_pktio_t pktio);

> >>

> >>   /**

> >> + * Start statistics for pktio handle

> >> + *

> >> + * @param	pktio	 Packet IO handle

> >> + * @retval  0 on success

> >> + * @retval <0 on failure

> >> + *

> >> + */

> >> +int odp_pktio_stats_start(odp_pktio_t pktio);

> >> +

> >> +/**

> >> + * Stop statistics for pktio handle

> >> + *

> >> + * @param	pktio	 Packet IO handle

> >> + * @retval  0 on success

> >> + * @retval <0 on failure

> >> + *

> >> + */

> >> +int odp_pktio_stats_stop(odp_pktio_t pktio);

> >> +

> >> +/**

> >>    * @}

> >>    */

> >>

> >> --

> >> 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

> >

> 

> --

> Regards,

> Ivan Khoronzhuk
Ivan Khoronzhuk Oct. 23, 2015, 11:11 a.m. | #4
On 23.10.15 14:03, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
> User tells to the implementation on open() which interfaces needed statistics.
>  As long as implementation has enough stat resources open() succeeds.
>  When all stats are gone and user still ask for stats, open() fails.
>Implementation user manual documents this limitation (how many interfaces can be
>  opened with stats enabled and in which combination).
>
> -Petri

Is it absolutely required for pktio to have statistic?
If no, then I can disable statistic for pktio after some test and use it for another port.
And closing pktio is not needed....

>> -----Original Message-----
>> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]
>> Sent: Friday, October 23, 2015 2:00 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-
>> odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start
>> and stop
>>
>>
>>
>> On 23.10.15 13:35, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>
>>> I think these are not generally needed or supported. It's better to add a
>> parameter into odp_pktio_param_t.
>>
>> It's needed.
>> The main reason for that the same statistic module can be used by different
>> pktios, so if it's used for one of them it cannot be used by other, to
>> allow
>> it for first it should be disable for second. For instance 2 statistic
>> modules
>> are shared between 4 eth ports, etc.
>>
>>>
>>> For example,
>>>
>>> typedef enum odp_pktio_stats_mode_t {
>>>
>>> /** Need basic statistics on this interface */
>>> ODP_PKTIO_STATS_BASIC = 0,
>>> /** Don't need any statistics on this interface */
>>> ODP_PKTIO_STATS_DISABLED
>>> } odp_pktio_stats_mode_t;
>>>
>>>
>>> -Petri
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>>>> Maxim Uvarov
>>>> Sent: Thursday, October 22, 2015 1:45 PM
>>>> To: lng-odp@lists.linaro.org
>>>> Subject: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start and
>>>> stop
>>>>
>>>> Define pktio stats start/stop functions for case when same
>>>> statistic module is used by different pktios, so if it's used
>>>> for one of them it cannot be used by other, to allow it for
>>>> first it should be disable for second. For instance 2 statistic
>>>> modules are shared between 4 eth ports, etc.
>>>>
>>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>>>> ---
>>>>    include/odp/api/packet_io_stats.h | 20 ++++++++++++++++++++
>>>>    1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/include/odp/api/packet_io_stats.h
>>>> b/include/odp/api/packet_io_stats.h
>>>> index 1bff9ca..03f060e 100644
>>>> --- a/include/odp/api/packet_io_stats.h
>>>> +++ b/include/odp/api/packet_io_stats.h
>>>> @@ -123,6 +123,26 @@ int odp_pktio_stats(odp_pktio_t pktio,
>>>>    int odp_pktio_stats_reset(odp_pktio_t pktio);
>>>>
>>>>    /**
>>>> + * Start statistics for pktio handle
>>>> + *
>>>> + * @param	pktio	 Packet IO handle
>>>> + * @retval  0 on success
>>>> + * @retval <0 on failure
>>>> + *
>>>> + */
>>>> +int odp_pktio_stats_start(odp_pktio_t pktio);
>>>> +
>>>> +/**
>>>> + * Stop statistics for pktio handle
>>>> + *
>>>> + * @param	pktio	 Packet IO handle
>>>> + * @retval  0 on success
>>>> + * @retval <0 on failure
>>>> + *
>>>> + */
>>>> +int odp_pktio_stats_stop(odp_pktio_t pktio);
>>>> +
>>>> +/**
>>>>     * @}
>>>>     */
>>>>
>>>> --
>>>> 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
>>>
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
Savolainen, Petri (Nokia - FI/Espoo) Oct. 23, 2015, 11:18 a.m. | #5
> -----Original Message-----

> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]

> Sent: Friday, October 23, 2015 2:12 PM

> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-

> odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start

> and stop

> 

> 

> 

> On 23.10.15 14:03, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >

> > User tells to the implementation on open() which interfaces needed

> statistics.

> >  As long as implementation has enough stat resources open() succeeds.

> >  When all stats are gone and user still ask for stats, open() fails.

> >Implementation user manual documents this limitation (how many interfaces

> can be

> >  opened with stats enabled and in which combination).

> >

> > -Petri

> 

> Is it absolutely required for pktio to have statistic?

> If no, then I can disable statistic for pktio after some test and use it

> for another port.

> And closing pktio is not needed....


It's not very useful to gather statistics part time. Either you need stats from an interface and want to count all packets (define ODP_PKTIO_STATS_BASIC), or you don’t need those at all (define ODP_PKTIO_STATS_DISABLED). What is the use case to time slice stats counting between two interfaces and potentially see zero packets on both, while packets are actually send and received (while you are measuring the other interface).

-Petri


> 

> >> -----Original Message-----

> >> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]

> >> Sent: Friday, October 23, 2015 2:00 PM

> >> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-

> >> odp@lists.linaro.org

> >> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start

> >> and stop

> >>

> >>

> >>

> >> On 23.10.15 13:35, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >>>

> >>> I think these are not generally needed or supported. It's better to add

> a

> >> parameter into odp_pktio_param_t.

> >>

> >> It's needed.

> >> The main reason for that the same statistic module can be used by

> different

> >> pktios, so if it's used for one of them it cannot be used by other, to

> >> allow

> >> it for first it should be disable for second. For instance 2 statistic

> >> modules

> >> are shared between 4 eth ports, etc.

> >>

> >>>

> >>> For example,

> >>>

> >>> typedef enum odp_pktio_stats_mode_t {

> >>>

> >>> /** Need basic statistics on this interface */

> >>> ODP_PKTIO_STATS_BASIC = 0,

> >>> /** Don't need any statistics on this interface */

> >>> ODP_PKTIO_STATS_DISABLED

> >>> } odp_pktio_stats_mode_t;

> >>>

> >>>

> >>> -Petri

> >>>

> >>>
Ivan Khoronzhuk Oct. 23, 2015, 11:40 a.m. | #6
On 23.10.15 14:18, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>
>> -----Original Message-----
>> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]
>> Sent: Friday, October 23, 2015 2:12 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-
>> odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start
>> and stop
>>
>>
>>
>> On 23.10.15 14:03, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>
>>> User tells to the implementation on open() which interfaces needed
>> statistics.
>>>   As long as implementation has enough stat resources open() succeeds.
>>>   When all stats are gone and user still ask for stats, open() fails.
>>> Implementation user manual documents this limitation (how many interfaces
>> can be
>>>   opened with stats enabled and in which combination).
>>>
>>> -Petri
>>
>> Is it absolutely required for pktio to have statistic?
>> If no, then I can disable statistic for pktio after some test and use it
>> for another port.
>> And closing pktio is not needed....
>
> It's not very useful to gather statistics part time.
If someone will find it usefull it can be added.
It doesn't corrupt any part of existent API.

>  Either you need stats from an interface and want to count all packets (define ODP_PKTIO_STATS_BASIC),
>or you don’t need those at all (define ODP_PKTIO_STATS_DISABLED).
>  What is the use case to time slice stats counting between two interfaces and potentially see zero packets on both,
>  while packets are actually send and received (while you are measuring the other interface).
Any comparison. Just using limited number of stat modules with pktios,
number of pktios can be more, like in my case.

>
> -Petri
>
>
>>
>>>> -----Original Message-----
>>>> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]
>>>> Sent: Friday, October 23, 2015 2:00 PM
>>>> To: Savolainen, Petri (Nokia - FI/Espoo); EXT Maxim Uvarov; lng-
>>>> odp@lists.linaro.org
>>>> Subject: Re: [lng-odp] [PATCHv5 2/2] api: pktio statistics: define start
>>>> and stop
>>>>
>>>>
>>>>
>>>> On 23.10.15 13:35, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>>>
>>>>> I think these are not generally needed or supported. It's better to add
>> a
>>>> parameter into odp_pktio_param_t.
>>>>
>>>> It's needed.
>>>> The main reason for that the same statistic module can be used by
>> different
>>>> pktios, so if it's used for one of them it cannot be used by other, to
>>>> allow
>>>> it for first it should be disable for second. For instance 2 statistic
>>>> modules
>>>> are shared between 4 eth ports, etc.
>>>>
>>>>>
>>>>> For example,
>>>>>
>>>>> typedef enum odp_pktio_stats_mode_t {
>>>>>
>>>>> /** Need basic statistics on this interface */
>>>>> ODP_PKTIO_STATS_BASIC = 0,
>>>>> /** Don't need any statistics on this interface */
>>>>> ODP_PKTIO_STATS_DISABLED
>>>>> } odp_pktio_stats_mode_t;
>>>>>
>>>>>
>>>>> -Petri
>>>>>
>>>>>

Patch

diff --git a/include/odp/api/packet_io_stats.h b/include/odp/api/packet_io_stats.h
index 1bff9ca..03f060e 100644
--- a/include/odp/api/packet_io_stats.h
+++ b/include/odp/api/packet_io_stats.h
@@ -123,6 +123,26 @@  int odp_pktio_stats(odp_pktio_t pktio,
 int odp_pktio_stats_reset(odp_pktio_t pktio);
 
 /**
+ * Start statistics for pktio handle
+ *
+ * @param	pktio	 Packet IO handle
+ * @retval  0 on success
+ * @retval <0 on failure
+ *
+ */
+int odp_pktio_stats_start(odp_pktio_t pktio);
+
+/**
+ * Stop statistics for pktio handle
+ *
+ * @param	pktio	 Packet IO handle
+ * @retval  0 on success
+ * @retval <0 on failure
+ *
+ */
+int odp_pktio_stats_stop(odp_pktio_t pktio);
+
+/**
  * @}
  */