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(&ethreq, 0, sizeof(ethreq));
>>>         strncpy(ethreq.ifr_name, netdev, IFNAMSIZ);
>>> -       ret = ioctl(pkt_sock->sockfd, SIOCGIFHWADDR, &ethreq);
>>> +       ret = ioctl(sockfd, SIOCGIFHWADDR, &ethreq);
>>>         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

Reply via email to