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]
> wrote:

>
>
>
> 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