[API-NEXT,PATCHv3] api: pktio link

Message ID 1449674483-13657-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Dec. 9, 2015, 3:21 p.m.
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 include/odp/api/packet_io.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Bill Fischofer Dec. 9, 2015, 3:26 p.m. | #1
On Wed, Dec 9, 2015 at 9:21 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

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

> ---

>  include/odp/api/packet_io.h | 11 +++++++++++

>  1 file changed, 11 insertions(+)

>

> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h

> index 443841e..0df46ba 100644

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

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

> @@ -673,6 +673,17 @@ void

> odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t *param);

>  void odp_pktio_print(odp_pktio_t pktio);

>

>  /**

> + * Determine if pktio link is up or down for a packet IO interface.

> + *

> + * @param[in] pktio Packet IO handle.

> + *

> + * @retval  1 link is up

> + * @retval  0 link is down

> + * @retval <0 on failure

> +*/

> +int odp_pktio_link(odp_pktio_t pktio);

>


It's not obvious from the name what this does.  Perhaps
odp_pktio_link_state() might be clearer?


> +

> +/**

>   * @}

>   */

>

> --

> 1.9.1

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Maxim Uvarov Dec. 9, 2015, 4:09 p.m. | #2
On 12/09/2015 18:26, Bill Fischofer wrote:
>
>
> On Wed, Dec 9, 2015 at 9:21 AM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>     ---
>      include/odp/api/packet_io.h | 11 +++++++++++
>      1 file changed, 11 insertions(+)
>
>     diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
>     index 443841e..0df46ba 100644
>     --- a/include/odp/api/packet_io.h
>     +++ b/include/odp/api/packet_io.h
>     @@ -673,6 +673,17 @@ void
>     odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t
>     *param);
>      void odp_pktio_print(odp_pktio_t pktio);
>
>      /**
>     + * Determine if pktio link is up or down for a packet IO interface.
>     + *
>     + * @param[in] pktio Packet IO handle.
>     + *
>     + * @retval  1 link is up
>     + * @retval  0 link is down
>     + * @retval <0 on failure
>     +*/
>     +int odp_pktio_link(odp_pktio_t pktio);
>
>
> It's not obvious from the name what this does.  Perhaps 
> odp_pktio_link_state() might be clearer?

state might be also stop/start or even speed and etc.

how about

odp_pktio_linkup()?

>     +
>     +/**
>       * @}
>       */
>
>     --
>     1.9.1
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Ola Liljedahl Dec. 9, 2015, 4:23 p.m. | #3
On 9 December 2015 at 17:09, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> On 12/09/2015 18:26, Bill Fischofer wrote:

>

>>

>>

>> On Wed, Dec 9, 2015 at 9:21 AM, Maxim Uvarov <maxim.uvarov@linaro.org

>> <mailto:maxim.uvarov@linaro.org>> wrote:

>>

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

>>     <mailto:maxim.uvarov@linaro.org>>

>>     ---

>>      include/odp/api/packet_io.h | 11 +++++++++++

>>      1 file changed, 11 insertions(+)

>>

>>     diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h

>>     index 443841e..0df46ba 100644

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

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

>>     @@ -673,6 +673,17 @@ void

>>     odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t

>>     *param);

>>      void odp_pktio_print(odp_pktio_t pktio);

>>

>>      /**

>>     + * Determine if pktio link is up or down for a packet IO interface.

>>     + *

>>     + * @param[in] pktio Packet IO handle.

>>     + *

>>     + * @retval  1 link is up

>>     + * @retval  0 link is down

>>     + * @retval <0 on failure

>>     +*/

>>     +int odp_pktio_link(odp_pktio_t pktio);

>>

>>

>> It's not obvious from the name what this does.  Perhaps

>> odp_pktio_link_state() might be clearer?

>>

>

> state might be also stop/start or even speed and etc.

>

> how about

>

> odp_pktio_linkup()?

>

-1

odp_pktio_link_state() is much better. Using 1 for up and 0 for down is
common sense (analogous to using 1 for true and 0 for false).


>

>     +

>>     +/**

>>       * @}

>>       */

>>

>>     --

>>     1.9.1

>>

>>     _______________________________________________

>>     lng-odp mailing list

>>     lng-odp@lists.linaro.org <mailto: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

>

Patch

diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
index 443841e..0df46ba 100644
--- a/include/odp/api/packet_io.h
+++ b/include/odp/api/packet_io.h
@@ -673,6 +673,17 @@  void odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t *param);
 void odp_pktio_print(odp_pktio_t pktio);
 
 /**
+ * Determine if pktio link is up or down for a packet IO interface.
+ *
+ * @param[in] pktio Packet IO handle.
+ *
+ * @retval  1 link is up
+ * @retval  0 link is down
+ * @retval <0 on failure
+*/
+int odp_pktio_link(odp_pktio_t pktio);
+
+/**
  * @}
  */