On 16 October 2015 at 17:55, Maxim Uvarov <maxim.uva...@linaro.org> wrote:

> On 10/16/2015 13:24, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>> RFC 3635 helps also here to define properties of an Ethernet-like
>> interface. It's not so as crucial to follow RFC terms and definitions here
>> as it's with counters, but we can use it as a guide line. Addresses and
>> promisc mode are classification issues. Name and pktio status (stopped /
>> started) ( == ifAdminStatus) are pktio level data.
>>
>>
>> typedef struct odp_pktio_link_state_t {
>>         uint8_t  type;        // 0: Ethernet (or enum
>> ODP_PKTIO_ETHERNET), ...
>>
>
> type is not state.  I think there might be odp_pktio_info_t which is
> filled by init of each pktio.
> and some call like:
>  const odp_pktio_info_t * odp_pktio_info(pktio);
>
> where
> odp_pktio_info_t {
>
> char *name;
> uint8_t  type;        // 0: Ethernet (or enum ODP_PKTIO_ETHERNET)
>
> // what also do we need, i/o address space, number of hw queues, nodes and
> etc.
> }
>
> But it looks like Bill right pointed that for now we have separate call
> for each function.
> We already have mtu and name for example. So we need to understand if we
> are going
> to modify existence API or add new.
>
> Also I can understand that:
> 1. speed -  might be needed to set up some timeout setting.
> 2.  status link up/down and logical pktio started / stopped is needed to
> check why odp_schedule() returns timeouts instead of packets.
>
> But I have no idea why application needs type, autoneg and duplex.
>
> I think for first try we need to stay just with:
> +typedef struct odp_pktio_link_state_t {
> +       uint32_t speed;         /**< Speed in Mbps: 10, 100, 1000 etc */
> +       odp_bool_t up;          /**< 1 - link up, 0 - link down */
> +       odp_bool_t stopped;     /**< 1 - pktio stopped, 0 - started */
>
Again, "stopped" is a poor name. Why not "running"?


> +} odp_pktio_link_state_t;
>
> I want to have up and stopped in one struct to speed that data plane calls.
>
Are these calls performance critical?



> Most likely it will be the following check
>
> ev = odp_schedule(wait_time);
> if (ev == ODP_EVENT_INVALID) {
>    odp_pktio_link_state(pktio, &link_state);
>    if (!link_state.up || link_state.stopped) {
>                 <send something to control plane,  >
>                   <or just wait for link, or pktio started>
>                   continue;
>     } else {
>         <just timeout, packet can be in any moment, ready to get them>
>         continue;
>   }
> }
>
> I.e. try to not do 2 calls and roll stack for 2 calls.
>
The example up doesn't look like it requires (or even benefits from) link
state and interface status to be obtained in the absolutely shortest time,
they are not called when we are actually processing an event.


> Does that reasonable?

No.

Interface (pktio) and physical link are different concepts which I do not
think should be mixed.



>
>
> Maxim.
>
>
>
>         uint8_t  status;      // 1: link up, 0: down, -1 not_present (==
>> ifOperStatus )
>>         uint8_t  autoneg;     // 1: autoneg on, 0: off, -1: not supported
>>         uint8_t  duplex;      // 1: full duplex, 0: half duplex
>>         uint32_t speed_mbps;  // Speed in Mbps. This leaves room for
>>                                    // speed_bps which would support odd
>> (Mbps) link speeds
>>         uint32_t mtu;         // MTU in bytes
>> } odp_pktio_link_state_t;
>>
>>
>> Better to use int (uint8_t) than bool to enable extendable status
>> definitions. E.g. ifOperStatus == not_present means that some HW component
>> is missing, rather than user have configured link down.
>>
>>
>> -Petri
>>
>>
>>
>> -----Original Message-----
>>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
>>> Maxim Uvarov
>>> Sent: Tuesday, October 13, 2015 3:39 PM
>>> To: lng-odp@lists.linaro.org
>>> Subject: [lng-odp] [API-NEXT PATCHv2] api: pktio link
>>>
>>> Define API to get pktio link state: seed, autoneg, link up/down,
>>> duplex, started/stopped state.
>>>
>>> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
>>> ---
>>>   v2: - use simple struct to return pktio link state;
>>>       - odp will not modify link, only ready it's state;
>>>
>>>
>>>   include/odp/api/packet_io.h | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
>>> index d8e69ed..6c77e8d 100644
>>> --- a/include/odp/api/packet_io.h
>>> +++ b/include/odp/api/packet_io.h
>>> @@ -96,6 +96,28 @@ typedef struct odp_pktio_param_t {
>>>   } odp_pktio_param_t;
>>>
>>>   /**
>>> + * Packet IO link state information
>>> + */
>>> +typedef struct odp_pktio_link_state_t {
>>> +       uint32_t speed;         /**< Speed in Mbps: 10, 100, 1000 etc */
>>> +       odp_bool_t up;          /**< 1 - link up, 0 - link down */
>>> +       odp_bool_t autoneg;     /**< 1 - autoneg on, 0 - off */
>>> +       odp_bool_t fd;          /**< 1 - full duplex, 0 - half duplex */
>>> +       odp_bool_t stopped;     /**< 1 - pktio stopped, 0 - started */
>>> +} odp_pktio_link_state_t;
>>> +
>>> +/**
>>> + * Get packet IO link state
>>> + *
>>> + * @param[in] pktio    Packet IO handle
>>> + * @param[out] state   Buffer to save link state
>>> + *
>>> + * @retval 0 on success (state info updated)
>>> + * @retval <0 on failure (state info not updated)
>>> + */
>>> +int odp_pktio_link_state(odp_pktio_t pktio, odp_pktio_link_state_t
>>> *state);
>>> +
>>> +/**
>>>    * Open a packet IO interface
>>>    *
>>>    * An ODP program can open a single packet IO interface per device,
>>> attempts
>>> --
>>> 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

Reply via email to