Hi, I think this is what we talked on the call yesterday.
/** * Get the default MAC address of a packet IO interface. * * @param[in] id ODP packet IO handle. * @param[out] mac_addr Storage for MAC address of the packet IO interface. * @param[in] addr_size Storage size for the address * * @retval Address size written, 0 on failure */ size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, size_t addr_size); -Petri From: [email protected] [mailto:[email protected]] On Behalf Of ext Bala Manoharan Sent: Thursday, November 27, 2014 10:09 AM To: Maxim Uvarov Cc: [email protected] Subject: Re: [lng-odp] [PATCH 3/5] pktio: mac addr functions Hi, I agree with Victor's concern, implementation needs a mechanism to know what is the amount of valid memory available in the mac_addr pointer. If I am not wrong the idea of defining the ODP_PKTIO_MAC_ADDR_MAX_LEN was initially discussed but the same was dropped as there were concerns since this value was an implementation detail and should be left outside of the API definition. Maybe we can have the following definition for odp_pktio_mac_addr +/** + * Get the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[in/out] addr_size Memory available for storage / Size of the MAC address + * @param[out] mac_addr Storage for MAC address of the packet IO interface. + * + * @retval -1 on any error. + * @retval -2 if the specified storage in mac_addr is not enough + * @retval 0 on Success. + */ +int odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_add, size_t *addr_size); The value addr_size could be an in/out parameter in the sense that the application while calling the API can specify memory available in the mac_addr pointer and the implementation can return the size of the mac address which gets filled. If the addr_size if smaller than the mac address of the interface the implementation can indicate the same using negative return value. Regards, Bala On 27 November 2014 at 04:05, Maxim Uvarov <[email protected]> wrote: On 11/27/2014 12:43 AM, Victor Kamensky wrote: On 26 November 2014 at 13:19, Maxim Uvarov <[email protected]> wrote: On 11/26/2014 09:00 PM, Victor Kamensky wrote: On 26 November 2014 at 09:31, Maxim Uvarov <[email protected]> wrote: Define API for mac address change and implement linux-generic version. Signed-off-by: Maxim Uvarov <[email protected]> --- platform/linux-generic/include/api/odp_packet_io.h | 23 ++++++++ platform/linux-generic/odp_packet_io.c | 67 ++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h index 20425be..480d930 100644 --- a/platform/linux-generic/include/api/odp_packet_io.h +++ b/platform/linux-generic/include/api/odp_packet_io.h @@ -173,6 +173,29 @@ int odp_pktio_promisc_set(odp_pktio_t id, odp_bool enable); int odp_pktio_promisc_enabled(odp_pktio_t id); /** + * Set the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[in] mac_addr MAC address to be assigned to the interface. + * @param[in] addr_size Size of the address in bytes. + * + * @return 0 on success, -1 on error. + */ +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char *mac_addr, + size_t addr_size); + +/** + * Get the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[out] mac_addr Storage for MAC address of the packet IO interface. How user knows what size should be allocated for this storage? Note if it is assumed to be fixed size, documentation should say so. But such approach would be inconsitent with odp_pktio_mac_addr_set function which does pass size of mac_addr storage to set. Maybe you want to pass size that user allocated for mac_addr storage and use it to copy result while returning real size of mac_addr, so user can compare whether it got all of it, and provide storage with required size if not. Thanks, Victor how about adding note that memory len for mac addr should use: #define ODP_PKTIO_MAC_ADDR_MAX_LEN 8 ? It would not address my point about inconsistency between odp_pktio_mac_addr, and odp_pktio_mac_addr_set function. Why one has 'size' parameter, and another does not. Also if user always must past mac_addr pointer to storage size of ODP_PKTIO_MAC_ADDR_MAX_LEN, would it not be better to have its type as 'const unsigned char[ODP_PKTIO_MAC_ADDR_MAX]_LEN'? Thanks, Victor Understand your point now. Now I think that it's better for implementation to provide storage for mac address. It might be part of pktio_entry. So current call might be: int odp_pktio_mac_addr(odp_pktio_t id, unsigned char **mac_addr); Or even better we can provide to api hole pktio_enty struct. I.e. struct odp_pktio_entry * odp_pktio_get(id); And this return entry will have eveything for current pktio: name, mac, promisc mode and etc. So that we will have one function for everything. And it's will be linked to packet i/o handle. So we know when handle is freed all this data can not be accessed. But that might be too late for odp 1.0. And I don't want to delay with more round of review / discussion. Might be const unsigned char[ODP_PKTIO_MAC_ADDR_MAX_LEN] is good for now. Maxim. Maxim. + * + * @retval -1 on any error. + * @retval size of mac addr. + */ +int odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr); + +/** * @} */ diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index b1dbc41..72531b3 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -21,6 +21,7 @@ #include <string.h> #include <sys/ioctl.h> +#include <linux/if_arp.h> typedef struct { pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; @@ -616,3 +617,69 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) else return 0; } + +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char *mac_addr, + size_t addr_size) +{ + pktio_entry_t *entry; + int sockfd; + struct ifreq ifr; + int ret; + + entry = get_entry(id); + if (entry == NULL) { + ODP_DBG("pktio entry %d does not exist\n", id); + return -1; + } + + if (entry->s.pkt_sock_mmap.sockfd) + sockfd = entry->s.pkt_sock_mmap.sockfd; + else + sockfd = entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); + ifr.ifr_name[IFNAMSIZ] = 0; + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; + + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); + if (ret < 0) { + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); + return -1; + } + + return 0; +} + +int odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr) +{ + pktio_entry_t *entry; + int sockfd; + struct ifreq ifr; + int ret; + + entry = get_entry(id); + if (entry == NULL) { + ODP_DBG("pktio entry %d does not exist\n", id); + return -1; + } + + if (entry->s.pkt_sock_mmap.sockfd) + sockfd = entry->s.pkt_sock_mmap.sockfd; + else + sockfd = entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); + ifr.ifr_name[IFNAMSIZ] = 0; + + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); + if (ret < 0) { + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); + return -1; + } + + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, + ETH_ALEN); + + return ETH_ALEN; +} -- 1.8.5.1.163.gd7aced9 _______________________________________________ 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
