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] > 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]> 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]> wrote: >> >>> >>> >>> >>> On 19 August 2014 10:13, Bala Manoharan <[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]> 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]> wrote: >>>>> >>>>>> Regards, >>>>>> Bala >>>>>> Signed-off-by: Balasubramanian Manoharan <[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(ðreq, 0, sizeof(ethreq)); >>>>>> strncpy(ethreq.ifr_name, netdev, IFNAMSIZ); >>>>>> - ret = ioctl(pkt_sock->sockfd, SIOCGIFHWADDR, ðreq); >>>>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, ðreq); >>>>>> 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] >>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>> >>>>> >>>>> >>>> >>> >> >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
