I'll add this topic to the agenda for today's ODP call as Bala suggests. Bill
On Tue, Aug 19, 2014 at 5:22 AM, Maxim Uvarov <[email protected]> wrote: > 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(ð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] >> <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 >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
