On 10/19/2015 18:39, Ola Liljedahl wrote:
On 19 October 2015 at 17:33, Maxim Uvarov <[email protected] <mailto:[email protected]>> wrote:

    On 10/19/2015 18:20, Ola Liljedahl wrote:

        On 16 October 2015 at 17:55, Maxim Uvarov
        <[email protected] <mailto:[email protected]>
        <mailto:[email protected]
        <mailto:[email protected]>>> 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"?


    Just serious why "stopped" is poor name and "running" is good name?

Because "stopped" (or "started") relates to the action (e.g. stop or start) you requested in order to change the state.
We don't call the link state variable "upped" or "downed" etc.


got it, thanks.

    In future we might
    have several states for pktio, something like paused. More likely
    it will be better to have just
    state which is some state enum.

            +} odp_pktio_link_state_t;

Yes better.


            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.


Ok, if there is no reason, then I will send patch only with up/down and speed.

Good.


    Maxim.



            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:[email protected]
        <mailto:[email protected]>
                    <mailto:[email protected]
        <mailto:[email protected]>>] On Behalf Of EXT
                    Maxim Uvarov
                    Sent: Tuesday, October 13, 2015 3:39 PM
                    To: [email protected]
        <mailto:[email protected]>
        <mailto:[email protected]
        <mailto:[email protected]>>
                    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
        <[email protected] <mailto:[email protected]>
                    <mailto:[email protected]
        <mailto:[email protected]>>>
                    ---
                      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
        [email protected] <mailto:[email protected]>
        <mailto:[email protected]
        <mailto:[email protected]>>
        https://lists.linaro.org/mailman/listinfo/lng-odp


            _______________________________________________
            lng-odp mailing list
        [email protected] <mailto:[email protected]>
        <mailto:[email protected]
        <mailto:[email protected]>>
        https://lists.linaro.org/mailman/listinfo/lng-odp





_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to