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
