Hi Ola, Since odp_pktio_t is an abstract type shouldn't the size of the mac address (bytes) be returned by the implementation rather than the application. The Application can give a byte array of size MAX_MAC_ADDRESS and the implementation can return the size of the mac address of which belongs to this current interface.
Regards, Bala On 4 September 2014 19:50, Ola Liljedahl <[email protected]> wrote: > I would still use a byte array, not a 64-bit integer. The application > specifies the size of the byte array (buffer) being passed in the call, the > function will check whether the buffer is large enough to hold the MAC > address. This would replace the current implicit assumption that the byte > array is (at least) six bytes large. > > > > On 4 September 2014 15:57, Savolainen, Petri (NSN - FI/Espoo) < > [email protected]> wrote: > >> If uint64_t would be used for mac, a set of functions would be needed >> to operate on it (e.g. copy it into a packet header, or check the bcast >> bit). Then a odp_eth_mac_t type would be better choice to the public API >> (instead of helpers). >> >> >> >> -Petri >> >> >> >> >> >> *From:* [email protected] [mailto: >> [email protected]] *On Behalf Of *ext Ola Liljedahl >> *Sent:* Thursday, September 04, 2014 4:01 PM >> *To:* Bala Manoharan; [email protected] >> *Subject:* Re: [lng-odp] [PATCH v3 1/1] API support for querying mac >> address >> >> >> >> I think the prototype should include the size of the buffer as well. >> Other types of network interfaces use 64-bit MAC addresses. The cost of >> looking forward is negligible. >> >> >> >> /** >> * Get MAC address of the interface >> * >> * @param hdl ODP packet IO handle >> * @param mac_addr Storage for MAC address of the packet IO interface >> (filled by function) >> >> * @param buf_size Size of mac_addr buffer. >> >> >> >> * @return Number of bytes written on success or -1 on error >> **/ >> int odp_pktio_get_mac_addr(odp_pktio_t hdl, unsigned char *mac_addr, >> size_t buf_size); >> >> >> >> >> >> On 4 September 2014 14:51, Bala Manoharan <[email protected]> >> wrote: >> >> FYI >> >> >> >> ---------- Forwarded message ---------- >> From: *Balasubramanian Manoharan* <[email protected]> >> Date: 20 August 2014 15:50 >> Subject: [lng-odp] [PATCH v3 1/1] API support for querying mac address >> To: [email protected] >> Cc: Balasubramanian Manoharan <[email protected]> >> >> >> This patch provides API support for querying mac address of an interface >> using odp_pktio_t handle. >> This current patch incorporates the review comments from the previous >> patch. >> >> The discussions are ongoing regarding adding additional API for querying >> mac address using device name, once it gets finalized the same will be >> provided as a different patch. >> >> 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 | 29 >> +++++++++++++++++++++- >> platform/linux-generic/odp_packet_netmap.c | 3 ++- >> platform/linux-generic/odp_packet_socket.c | 19 +++++++------- >> 5 files changed, 49 insertions(+), 11 deletions(-) >> >> diff --git a/include/odp_packet_io.h b/include/odp_packet_io.h >> index cfefac0..86778bf 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..69174fe 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..ddad7a4 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,30 @@ 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) { >> + ODP_ERR("Invalid odp_pktio_t value\n"); >> + return -1; >> + } >> + 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..13f3f52 100644 >> --- a/platform/linux-generic/odp_packet_netmap.c >> +++ b/platform/linux-generic/odp_packet_netmap.c >> @@ -222,9 +222,10 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, >> const char *netdev, >> ODP_ERR("Error: token creation failed\n"); >> return -1; >> } >> + if (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..593d093 100644 >> --- a/platform/linux-generic/odp_packet_socket.c >> +++ b/platform/linux-generic/odp_packet_socket.c >> @@ -57,6 +57,8 @@ typedef struct { >> >> static raw_socket_t raw_sockets[MAX_RAW_SOCKETS_NETDEVS]; >> static odp_spinlock_t raw_sockets_lock; >> +static int socket_store_hw_addr(int sockfd, unsigned char *if_mac, >> + const char *netdev); >> >> /** Eth buffer start offset from u32-aligned address to make sure the >> following >> * header (e.g. IP) starts at a 32-bit aligned address. >> @@ -153,7 +155,6 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock, const >> char *netdev, >> if (pool == ODP_BUFFER_POOL_INVALID) >> return -1; >> pkt_sock->pool = pool; >> - >> pkt = odp_packet_alloc(pool); >> if (!odp_packet_is_valid(pkt)) >> return -1; >> @@ -168,12 +169,14 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock, >> const char *netdev, >> pkt_sock->max_frame_len = pkt_sock->buf_size - >> pkt_sock->frame_offset; >> >> odp_packet_free(pkt); >> - >> odp_spinlock_lock(&raw_sockets_lock); >> - >> sockfd = find_raw_fd(netdev); >> if (sockfd) { >> pkt_sock->sockfd = sockfd; >> + if (socket_store_hw_addr(sockfd, pkt_sock->if_mac, >> netdev)) { >> + perror("setup_pkt_sock() - >> socket_store_hw_addr()"); >> + goto error; >> + } >> odp_spinlock_unlock(&raw_sockets_lock); >> return sockfd; >> } >> @@ -215,7 +218,6 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock, const >> char *netdev, >> perror("setup_pkt_sock() - bind(to IF)"); >> goto error; >> } >> - >> add_raw_fd(netdev, sockfd); >> odp_spinlock_unlock(&raw_sockets_lock); >> return sockfd; >> @@ -735,7 +737,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 +746,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,8 +807,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); >> - if (ret != 0) >> + if (socket_store_hw_addr(pkt_sock->sockfd, pkt_sock->if_mac, >> netdev)) >> return -1; >> >> if_idx = if_nametoindex(netdev); >> -- >> 2.0.1.472.g6f92e5f >> >> >> >> >> > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
