On 26 November 2014 at 13:19, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
> On 11/26/2014 09:00 PM, Victor Kamensky wrote:
>>
>> On 26 November 2014 at 09:31, Maxim Uvarov <maxim.uva...@linaro.org>
>> wrote:
>>>
>>> Define API for mac address change and implement linux-generic version.
>>>
>>> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
>>> ---
>>> 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
>
> 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
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
