Alex, what about same function to set mac address?
Ciprian needed this for his OVS work. (If it's still true).

Maxim.

On 08/19/2014 02:15 PM, Alexandru Badicioiu wrote:
Please correct me if I'm wrong, but mac_addr and pktio are not on the same level of abstraction - mac_addr is not abstract and is something specific to an Ethernet MAC device (which in fact can have more than one MAC address assigned). This doesn't vary from platform to platform (I guess), but pktio does.
Having available this call only:
int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char* mac_addr);
implies that any MAC address must be derived from an pktio which imposes a constraint on what a pktio represents or how it is implemented.

Alex


On 19 August 2014 12:13, Bala Manoharan <[email protected] <mailto:[email protected]>> wrote:

    Hi,

    If we add this additional API odp_get_mac_addr(const char* dev,
    char* mac_addr) then we need to have a limitation that this API
    cannot be called in case of bare-metal implementation before
    calling odp_pktio_open().

    The reason I am sceptical about this API is that this provides a
    means to access the mac_addr of an interface without using the
    odp_pktio_t handle which defeats the purpose of abstraction. But
    if there is a consensus on this new API then I can add the same
    and provide a patch.

    We can discuss this topic in detail during todays ODP call.

    Regards,
    Bala


    On 19 August 2014 14:19, Alexandru Badicioiu
    <[email protected]
    <mailto:[email protected]>> wrote:

        I understand this bare metal use case where open() call
        initializes the port , but having the MAC address derived
        exclusively from a pktio assumes that this is meaningful for
        any implementation and the implementation has a way to do this.
        This is why I think there should be also an API call for
        getting the MAC address from a specific name.

        Alex


        On 19 August 2014 11:30, Bala Manoharan
        <[email protected] <mailto:[email protected]>>
        wrote:

            Hi,

            I agree with your point "const char* dev" need not point
            to a device, it can point to the location of the physical
            port which can be defined by the platform. In case of a
            bare-metal implementation the physical port will have to
            be initialized and mac address needs to be assigned to the
            port during initialization. Hence odp_pktio_open() call
            can be used to initialize the physical port and once the
            port gets initialized the mac_address of the port can be
            read using pktio handle.

            Pls let me know if I am missing some points from your
            description.

            Regards,
            Bala


            On 19 August 2014 13:37, Alexandru Badicioiu
            <[email protected]
            <mailto:[email protected]>> wrote:




                On 19 August 2014 10:13, Bala Manoharan
                <[email protected]
                <mailto:[email protected]>> wrote:

                    Hi Alex,

                    I had the same thought while implementing this API.
                    The reason I decided to go ahead with getting mac
                    address only though odp_pktio_t is because if we
                    have this API to get mac address directly with the
                    device name then the user can try to get mac
                    address of a device even without creating the
                    PKTIO instance for the device.

                    Which could have an issue in the systems where
                    some initialization is done during PKTIO creation.
                    Also the check for validating the device name
                    could be kept in odp_pktio_open() call alone.

                Maybe I'm not aware of all the cases, but in Linux
                usually you don't need to bring up an interface (which
                triggers the netdev_ops open() call) to get the MAC
                address of the Ethernet port.
                However, there is no restriction that *dev argument of
                pktio_open() should denote an Ethernet interface or to
                be possible to derive a single Ethernet port from it.
                I think in this case a (const char *dev, char
                *mac_addr) function is mandatory.

                Alex

                    Regards,
                    Bala


                    On 19 August 2014 12:37, Alexandru Badicioiu
                    <[email protected]
                    <mailto:[email protected]>> wrote:

                        Hi,
                        from the implementation of the proposed API
                        call it looks like the intent of the API
                        function is to get the MAC address of the
                        device used to open the pktio.
                        From the documentation it looks like the *dev
                        is the actual IO device:
                        /**
                         * Open an ODP packet IO instance
                         *
                         * @param dev    Packet IO device
                         * @param pool   Pool to use for packet IO
                         * @param params Set of parameters to pass to
                        the arch dependent implementation
                         *
                         * @return ODP packet IO handle or
                        ODP_PKTIO_INVALID on error
                         */
                        odp_pktio_t odp_pktio_open(const char *dev,
                        odp_buffer_pool_t pool,
                         odp_pktio_params_t *params);

                        Should we have also an API call :
                        int odp_get_mac_addr(const char *dev, unsigned
                        char* mac_addr) ?

                        Thanks,
                        Alex


                        On 19 August 2014 08:56, Balasubramanian
                        Manoharan <[email protected]
                        <mailto:[email protected]>> wrote:

                            Regards,
                            Bala
                            Signed-off-by: Balasubramanian Manoharan
                            <[email protected]
                            <mailto:[email protected]>>
                            ---
                             include/odp_packet_io.h             | 8
                            +++++++
                             platform/linux-generic/include/odp_packet_netmap.h
                            |  1 +
platform/linux-generic/odp_packet_io.c | 28 +++++++++++++++++++++- platform/linux-generic/odp_packet_netmap.c | 4 +++- platform/linux-generic/odp_packet_socket.c | 8 +++----
                             5 files changed, 43 insertions(+), 6
                            deletions(-)

                            diff --git a/include/odp_packet_io.h
                            b/include/odp_packet_io.h
                            index cfefac0..6c06520 100644
                            --- a/include/odp_packet_io.h
                            +++ b/include/odp_packet_io.h
                            @@ -129,6 +129,14 @@ void
                            odp_pktio_set_input(odp_packet_t pkt,
                            odp_pktio_t id);
                              */
                             odp_pktio_t
                            odp_pktio_get_input(odp_packet_t pkt);

                            +/**
                            + * Get mac address of the interface
                            + *
                            + * @param id          ODP packet IO handle
                            + * @param mac_addr  Storage for Mac
                            address of the packet IO interface (filled
                            by function)
                            + * @return  0 on success or -1 on error
                            +**/
                            +int odp_pktio_get_mac_addr(odp_pktio_t
                            id, unsigned char* mac_addr);
                             #ifdef __cplusplus
                             }
                             #endif
                            diff --git
                            a/platform/linux-generic/include/odp_packet_netmap.h
                            b/platform/linux-generic/include/odp_packet_netmap.h
                            index 57d9f2c..3693416 100644
                            ---
                            a/platform/linux-generic/include/odp_packet_netmap.h
                            +++
                            b/platform/linux-generic/include/odp_packet_netmap.h
                            @@ -40,6 +40,7 @@ typedef struct {
                            odp_queue_t tx_access; /* Used for
                            exclusive access to send packets */
                            uint32_t if_flags;
                                    char ifname[32];
                            +  unsigned char if_mac[ETH_ALEN];
                             } pkt_netmap_t;

                             /**
                            diff --git
                            a/platform/linux-generic/odp_packet_io.c
                            b/platform/linux-generic/odp_packet_io.c
                            index 33ade10..069b0e1 100644
                            --- a/platform/linux-generic/odp_packet_io.c
                            +++ b/platform/linux-generic/odp_packet_io.c
                            @@ -245,7 +245,7 @@ odp_pktio_t
                            odp_pktio_open(const char *dev,
                            odp_buffer_pool_t pool,
                            ODP_ERR("This type of I/O is not
                            supported. Please recompile.\n");
                              break;
                                    }
                            -
                            +  pktio_entry->s.params.type = params->type;
                            unlock_entry(pktio_entry);
                                    return id;
                             }
                            @@ -535,3 +535,29 @@ int
                            pktin_deq_multi(queue_entry_t *qentry,
                            odp_buffer_hdr_t *buf_hdr[], int num)

                                    return nbr;
                             }
                            +int odp_pktio_get_mac_addr(odp_pktio_t
                            pkt, unsigned char* mac_addr){
                            +
                            +  pktio_entry_t * pktio_entry =
                            get_entry(pkt);
                            +  if(!pktio_entry){
                            +  printf("Invalid odp_pktio_t value\n");
                            +  return ODP_PKTIO_INVALID;
                            +       }
                            +  switch(pktio_entry->s.params.type){
                            +  case ODP_PKTIO_TYPE_SOCKET_BASIC:
                            +  case ODP_PKTIO_TYPE_SOCKET_MMSG:
                            +  memcpy(mac_addr,
                            pktio_entry->s.pkt_sock.if_mac, ETH_ALEN);
                            +  break;
                            +  case ODP_PKTIO_TYPE_SOCKET_MMAP:
                            +  memcpy(mac_addr,
                            pktio_entry->s.pkt_sock_mmap.if_mac,
                            ETH_ALEN);
                            +  break;
                            +#ifdef ODP_HAVE_NETMAP
                            +  case ODP_PKTIO_TYPE_NETMAP:
                            +  memcpy(mac_addr,
                            pktio_entry->s.pkt_nm.if_mac, ETH_ALEN);
                            +  break;
                            +#endif
                            +  default:
                            +  ODP_ERR("Invalid pktio type: %02x\n",
                            pktio_entry->s.params.type);
                            +  return ODP_PKTIO_INVALID;
                            +       }
                            +       return 0;
                            +}
                            diff --git
                            a/platform/linux-generic/odp_packet_netmap.c
                            b/platform/linux-generic/odp_packet_netmap.c
                            index e2215ab..e9e9f56 100644
                            ---
                            a/platform/linux-generic/odp_packet_netmap.c
                            +++
                            b/platform/linux-generic/odp_packet_netmap.c
                            @@ -222,9 +222,11 @@ int
                            setup_pkt_netmap(pkt_netmap_t * const
                            pkt_nm, const char *netdev,
                            ODP_ERR("Error: token creation failed\n");
                              return -1;
                                    }
                            +       if( 0 !=
                            socket_store_hw_addr(pkt_nm->if_mac, netdev)
                            +   return -1;

                            odp_queue_enq(pkt_nm->tx_access, token);
                            -
                            +
                            ODP_DBG("Wait for link to come up\n");
                            sleep(WAITLINK_TMO);
                            ODP_DBG("Done\n");
                            diff --git
                            a/platform/linux-generic/odp_packet_socket.c
                            b/platform/linux-generic/odp_packet_socket.c
                            index d44c333..d96aa8e 100644
                            ---
                            a/platform/linux-generic/odp_packet_socket.c
                            +++
                            b/platform/linux-generic/odp_packet_socket.c
                            @@ -735,7 +735,7 @@ static int
                            mmap_bind_sock(pkt_sock_mmap_t *pkt_sock,
                            const char *netdev)
                                    return 0;
                             }

                            -static int
                            mmap_store_hw_addr(pkt_sock_mmap_t * const
                            pkt_sock,
                            +static int socket_store_hw_addr(int
                            sockfd,unsigned char * if_mac,
                              const char *netdev)
                             {
                                    struct ifreq ethreq;
                            @@ -744,13 +744,13 @@ static int
                            mmap_store_hw_addr(pkt_sock_mmap_t * const
                            pkt_sock,
                                    /* get MAC address */
                            memset(&ethreq, 0, sizeof(ethreq));
                            strncpy(ethreq.ifr_name, netdev, IFNAMSIZ);
                            -       ret = ioctl(pkt_sock->sockfd,
                            SIOCGIFHWADDR, &ethreq);
                            +       ret = ioctl(sockfd, SIOCGIFHWADDR,
                            &ethreq);
                                    if (ret != 0) {
                            perror("store_hw_addr() -
                            ioctl(SIOCGIFHWADDR)");
                              return -1;
                                    }

                            -  ethaddr_copy(pkt_sock->if_mac,
                            +  ethaddr_copy(if_mac,
                             (unsigned char
                            *)ethreq.ifr_ifru.ifru_hwaddr.sa_data);

                                    return 0;
                            @@ -805,7 +805,7 @@ int
                            setup_pkt_sock_mmap(pkt_sock_mmap_t *
                            const pkt_sock, const char *netdev,
                                    if (ret != 0)
                              return -1;

                            -       ret = mmap_store_hw_addr(pkt_sock,
                            netdev);
                            +       ret =
                            socket_store_hw_addr(pkt_sock->sockfd,
                            pkt_sock->if_mac, netdev);
                                    if (ret != 0)
                              return -1;

                            --
                            2.0.1.472.g6f92e5f


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










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


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

Reply via email to