diff mbox

[1/1] API support to get mac address of an interface using odp_pktio_t

Message ID 1408427763-30086-1-git-send-email-bala.manoharan@linaro.org
State New
Headers show

Commit Message

Balasubramanian Manoharan Aug. 19, 2014, 5:56 a.m. UTC 
Regards,
Bala
Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
---
 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(-)

Comments

Santosh Shukla Aug. 19, 2014, 6:10 a.m. UTC | #1 
On 19 August 2014 11:26, Balasubramanian Manoharan
<bala.manoharan@linaro.org> wrote:
> Regards,
> Bala

this is bad :) add proper description.

do git commit --amend
- remove above and add relevant description.
- save it, then review with "git log"

> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
> ---
>  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;

if (socket_store_hw_addr())
      return -1;

>
>         odp_queue_enq(pkt_nm->tx_access, token);
> -
> +

Why?
>         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
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Balasubramanian Manoharan Aug. 19, 2014, 6:16 a.m. UTC | #2 
Hi Santosh,


if (socket_store_hw_addr())
      return -1;

>>> We cannot use this since the return value for error will be changed to
-1 and not zero in the future.
I have kept this as of now since I believe petri is changing the #defines
for different error scenarios.

We have to check the return value using the following snippet,
if(ODP_PKTIO_INVALID != socket_store_hw_addr()){
          return -1;
}


On 19 August 2014 11:40, Santosh Shukla <santosh.shukla@linaro.org> wrote:

> On 19 August 2014 11:26, Balasubramanian Manoharan
> <bala.manoharan@linaro.org> wrote:
> > Regards,
> > Bala
>
> this is bad :) add proper description.
>
> do git commit --amend
> - remove above and add relevant description.
> - save it, then review with "git log"
>
> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
> > ---
> >  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;
>
> if (socket_store_hw_addr())
>       return -1;
>
> >
> >         odp_queue_enq(pkt_nm->tx_access, token);
> > -
> > +
>
> Why?
> >         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
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
Santosh Shukla Aug. 19, 2014, 6:17 a.m. UTC | #3 
On 19 August 2014 11:46, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> Hi Santosh,
>
>
> if (socket_store_hw_addr())
>       return -1;
>
>>>> We cannot use this since the return value for error will be changed to
>>>> -1 and not zero in the future.
> I have kept this as of now since I believe petri is changing the #defines
> for different error scenarios.
>
> We have to check the return value using the following snippet,
> if(ODP_PKTIO_INVALID != socket_store_hw_addr()){
>           return -1;
> }

Fine, then write explicit comment on top about your assumption. Thanks.
>
>
> On 19 August 2014 11:40, Santosh Shukla <santosh.shukla@linaro.org> wrote:
>>
>> On 19 August 2014 11:26, Balasubramanian Manoharan
>> <bala.manoharan@linaro.org> wrote:
>> > Regards,
>> > Bala
>>
>> this is bad :) add proper description.
>>
>> do git commit --amend
>> - remove above and add relevant description.
>> - save it, then review with "git log"
>>
>> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>> > ---
>> >  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;
>>
>> if (socket_store_hw_addr())
>>       return -1;
>>
>> >
>> >         odp_queue_enq(pkt_nm->tx_access, token);
>> > -
>> > +
>>
>> Why?
>> >         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
>> > lng-odp@lists.linaro.org
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Balasubramanian Manoharan Aug. 19, 2014, 6:24 a.m. UTC | #4 
Sure will do and add the comments in final patch.

Regards,
Bala


On 19 August 2014 11:47, Santosh Shukla <santosh.shukla@linaro.org> wrote:

> On 19 August 2014 11:46, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> > Hi Santosh,
> >
> >
> > if (socket_store_hw_addr())
> >       return -1;
> >
> >>>> We cannot use this since the return value for error will be changed to
> >>>> -1 and not zero in the future.
> > I have kept this as of now since I believe petri is changing the #defines
> > for different error scenarios.
> >
> > We have to check the return value using the following snippet,
> > if(ODP_PKTIO_INVALID != socket_store_hw_addr()){
> >           return -1;
> > }
>
> Fine, then write explicit comment on top about your assumption. Thanks.
> >
> >
> > On 19 August 2014 11:40, Santosh Shukla <santosh.shukla@linaro.org>
> wrote:
> >>
> >> On 19 August 2014 11:26, Balasubramanian Manoharan
> >> <bala.manoharan@linaro.org> wrote:
> >> > Regards,
> >> > Bala
> >>
> >> this is bad :) add proper description.
> >>
> >> do git commit --amend
> >> - remove above and add relevant description.
> >> - save it, then review with "git log"
> >>
> >> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
> >> > ---
> >> >  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;
> >>
> >> if (socket_store_hw_addr())
> >>       return -1;
> >>
> >> >
> >> >         odp_queue_enq(pkt_nm->tx_access, token);
> >> > -
> >> > +
> >>
> >> Why?
> >> >         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
> >> > lng-odp@lists.linaro.org
> >> > http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
>
Alexandru Badicioiu Aug. 19, 2014, 7:07 a.m. UTC | #5 
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 <
bala.manoharan@linaro.org> wrote:

> Regards,
> Bala
> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
> ---
>  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
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Balasubramanian Manoharan Aug. 19, 2014, 7:13 a.m. UTC | #6 
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.

Regards,
Bala


On 19 August 2014 12:37, Alexandru Badicioiu <alexandru.badicioiu@linaro.org
> 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 <
> bala.manoharan@linaro.org> wrote:
>
>> Regards,
>> Bala
>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>> ---
>>  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
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
Alexandru Badicioiu Aug. 19, 2014, 8:07 a.m. UTC | #7 
On 19 August 2014 10:13, Bala Manoharan <bala.manoharan@linaro.org> 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 <
> alexandru.badicioiu@linaro.org> 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 <
>> bala.manoharan@linaro.org> wrote:
>>
>>> Regards,
>>> Bala
>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>>> ---
>>>  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
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>
Balasubramanian Manoharan Aug. 19, 2014, 8:30 a.m. UTC | #8 
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 <alexandru.badicioiu@linaro.org
> wrote:

>
>
>
> On 19 August 2014 10:13, Bala Manoharan <bala.manoharan@linaro.org> 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 <
>> alexandru.badicioiu@linaro.org> 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 <
>>> bala.manoharan@linaro.org> wrote:
>>>
>>>> Regards,
>>>> Bala
>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>>>> ---
>>>>  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
>>>> lng-odp@lists.linaro.org
>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>
>>>
>>>
>>
>
Alexandru Badicioiu Aug. 19, 2014, 8:49 a.m. UTC | #9 
I understand this bare metal use case where open() call initializes the
port , but having the MAC address derived exclusively from a pktio assumes
that this is meaningful for any implementation and the implementation has a
way to do this.
This is why I think there should be also an API call for getting the MAC
address from a specific name.

Alex


On 19 August 2014 11:30, Bala Manoharan <bala.manoharan@linaro.org> wrote:

> 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 <
> alexandru.badicioiu@linaro.org> wrote:
>
>>
>>
>>
>> On 19 August 2014 10:13, Bala Manoharan <bala.manoharan@linaro.org>
>> 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 <
>>> alexandru.badicioiu@linaro.org> 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 <
>>>> bala.manoharan@linaro.org> wrote:
>>>>
>>>>> Regards,
>>>>> Bala
>>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>>>>> ---
>>>>>  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
>>>>> lng-odp@lists.linaro.org
>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>
>>>>
>>>>
>>>
>>
>
Balasubramanian Manoharan Aug. 19, 2014, 9:13 a.m. UTC | #10 
Hi,

If we add this additional API odp_get_mac_addr(const char* dev, char*
mac_addr) then we need to have a limitation that this API cannot be called
in case of bare-metal implementation before calling odp_pktio_open().

The reason I am sceptical about this API is that this provides a means to
access the mac_addr of an interface without using the odp_pktio_t handle
which defeats the purpose of abstraction. But if there is a consensus on
this new API then I can add the same and provide a patch.

We can discuss this topic in detail during todays ODP call.

Regards,
Bala


On 19 August 2014 14:19, Alexandru Badicioiu <alexandru.badicioiu@linaro.org
> wrote:

> I understand this bare metal use case where open() call initializes the
> port , but having the MAC address derived exclusively from a pktio assumes
> that this is meaningful for any implementation and the implementation has a
> way to do this.
> This is why I think there should be also an API call for getting the MAC
> address from a specific name.
>
> Alex
>
>
> On 19 August 2014 11:30, Bala Manoharan <bala.manoharan@linaro.org> wrote:
>
>> 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 <
>> alexandru.badicioiu@linaro.org> wrote:
>>
>>>
>>>
>>>
>>> On 19 August 2014 10:13, Bala Manoharan <bala.manoharan@linaro.org>
>>> 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 <
>>>> alexandru.badicioiu@linaro.org> 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 <
>>>>> bala.manoharan@linaro.org> wrote:
>>>>>
>>>>>> Regards,
>>>>>> Bala
>>>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>>>>>> ---
>>>>>>  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
>>>>>> lng-odp@lists.linaro.org
>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
Alexandru Badicioiu Aug. 19, 2014, 10:15 a.m. UTC | #11 
Please correct me if I'm wrong, but mac_addr and pktio are not on the same
level of abstraction -
mac_addr is not abstract and is something specific to an Ethernet MAC
device (which in fact can have more than one MAC address assigned). This
doesn't vary from platform to platform (I guess), but pktio does.
Having available this call only:
int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char* mac_addr);
implies that any MAC address must be derived from an pktio which imposes a
constraint on what a pktio represents or how it is implemented.

Alex


On 19 August 2014 12:13, Bala Manoharan <bala.manoharan@linaro.org> wrote:

> Hi,
>
> If we add this additional API odp_get_mac_addr(const char* dev, char*
> mac_addr) then we need to have a limitation that this API cannot be called
> in case of bare-metal implementation before calling odp_pktio_open().
>
> The reason I am sceptical about this API is that this provides a means to
> access the mac_addr of an interface without using the odp_pktio_t handle
> which defeats the purpose of abstraction. But if there is a consensus on
> this new API then I can add the same and provide a patch.
>
> We can discuss this topic in detail during todays ODP call.
>
> Regards,
> Bala
>
>
> On 19 August 2014 14:19, Alexandru Badicioiu <
> alexandru.badicioiu@linaro.org> wrote:
>
>> I understand this bare metal use case where open() call initializes the
>> port , but having the MAC address derived exclusively from a pktio assumes
>> that this is meaningful for any implementation and the implementation has a
>> way to do this.
>> This is why I think there should be also an API call for getting the MAC
>> address from a specific name.
>>
>> Alex
>>
>>
>> On 19 August 2014 11:30, Bala Manoharan <bala.manoharan@linaro.org>
>> wrote:
>>
>>> 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 <
>>> alexandru.badicioiu@linaro.org> wrote:
>>>
>>>>
>>>>
>>>>
>>>> On 19 August 2014 10:13, Bala Manoharan <bala.manoharan@linaro.org>
>>>> 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 <
>>>>> alexandru.badicioiu@linaro.org> 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 <
>>>>>> bala.manoharan@linaro.org> wrote:
>>>>>>
>>>>>>> Regards,
>>>>>>> Bala
>>>>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>>>>>>> ---
>>>>>>>  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
>>>>>>> lng-odp@lists.linaro.org
>>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Maxim Uvarov Aug. 19, 2014, 10:22 a.m. UTC | #12 
Alex, what about same function to set mac address?
Ciprian needed this for his OVS work. (If it's still true).

Maxim.

On 08/19/2014 02:15 PM, Alexandru Badicioiu wrote:
> Please correct me if I'm wrong, but mac_addr and pktio are not on the 
> same level of abstraction -
> mac_addr is not abstract and is something specific to an Ethernet MAC 
> device (which in fact can have more than one MAC address assigned). 
> This doesn't vary from platform to platform (I guess), but pktio does.
> Having available this call only:
> int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char* mac_addr);
> implies that any MAC address must be derived from an pktio which 
> imposes a constraint on what a pktio represents or how it is implemented.
>
> Alex
>
>
> On 19 August 2014 12:13, Bala Manoharan <bala.manoharan@linaro.org 
> <mailto:bala.manoharan@linaro.org>> wrote:
>
>     Hi,
>
>     If we add this additional API odp_get_mac_addr(const char* dev,
>     char* mac_addr) then we need to have a limitation that this API
>     cannot be called in case of bare-metal implementation before
>     calling odp_pktio_open().
>
>     The reason I am sceptical about this API is that this provides a
>     means to access the mac_addr of an interface without using the
>     odp_pktio_t handle which defeats the purpose of abstraction. But
>     if there is a consensus on this new API then I can add the same
>     and provide a patch.
>
>     We can discuss this topic in detail during todays ODP call.
>
>     Regards,
>     Bala
>
>
>     On 19 August 2014 14:19, Alexandru Badicioiu
>     <alexandru.badicioiu@linaro.org
>     <mailto:alexandru.badicioiu@linaro.org>> wrote:
>
>         I understand this bare metal use case where open() call
>         initializes the port , but having the MAC address derived
>         exclusively from a pktio assumes that this is meaningful for
>         any implementation and the implementation has a way to do this.
>         This is why I think there should be also an API call for
>         getting the MAC address from a specific name.
>
>         Alex
>
>
>         On 19 August 2014 11:30, Bala Manoharan
>         <bala.manoharan@linaro.org <mailto:bala.manoharan@linaro.org>>
>         wrote:
>
>             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
>             <alexandru.badicioiu@linaro.org
>             <mailto:alexandru.badicioiu@linaro.org>> wrote:
>
>
>
>
>                 On 19 August 2014 10:13, Bala Manoharan
>                 <bala.manoharan@linaro.org
>                 <mailto:bala.manoharan@linaro.org>> 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
>                     <alexandru.badicioiu@linaro.org
>                     <mailto:alexandru.badicioiu@linaro.org>> 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 <bala.manoharan@linaro.org
>                         <mailto:bala.manoharan@linaro.org>> wrote:
>
>                             Regards,
>                             Bala
>                             Signed-off-by: Balasubramanian Manoharan
>                             <bala.manoharan@linaro.org
>                             <mailto:bala.manoharan@linaro.org>>
>                             ---
>                              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
>                             lng-odp@lists.linaro.org
>                             <mailto: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
Bill Fischofer Aug. 19, 2014, 10:40 a.m. UTC | #13 
I'll add this topic to the agenda for today's ODP call as Bala suggests.

Bill


On Tue, Aug 19, 2014 at 5:22 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Alex, what about same function to set mac address?
> Ciprian needed this for his OVS work. (If it's still true).
>
> Maxim.
>
>
> On 08/19/2014 02:15 PM, Alexandru Badicioiu wrote:
>
>> Please correct me if I'm wrong, but mac_addr and pktio are not on the
>> same level of abstraction -
>> mac_addr is not abstract and is something specific to an Ethernet MAC
>> device (which in fact can have more than one MAC address assigned). This
>> doesn't vary from platform to platform (I guess), but pktio does.
>> Having available this call only:
>> int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char* mac_addr);
>> implies that any MAC address must be derived from an pktio which imposes
>> a constraint on what a pktio represents or how it is implemented.
>>
>> Alex
>>
>>
>> On 19 August 2014 12:13, Bala Manoharan <bala.manoharan@linaro.org
>> <mailto:bala.manoharan@linaro.org>> wrote:
>>
>>     Hi,
>>
>>     If we add this additional API odp_get_mac_addr(const char* dev,
>>     char* mac_addr) then we need to have a limitation that this API
>>     cannot be called in case of bare-metal implementation before
>>     calling odp_pktio_open().
>>
>>     The reason I am sceptical about this API is that this provides a
>>     means to access the mac_addr of an interface without using the
>>     odp_pktio_t handle which defeats the purpose of abstraction. But
>>     if there is a consensus on this new API then I can add the same
>>     and provide a patch.
>>
>>     We can discuss this topic in detail during todays ODP call.
>>
>>     Regards,
>>     Bala
>>
>>
>>     On 19 August 2014 14:19, Alexandru Badicioiu
>>     <alexandru.badicioiu@linaro.org
>>     <mailto:alexandru.badicioiu@linaro.org>> wrote:
>>
>>         I understand this bare metal use case where open() call
>>         initializes the port , but having the MAC address derived
>>         exclusively from a pktio assumes that this is meaningful for
>>         any implementation and the implementation has a way to do this.
>>         This is why I think there should be also an API call for
>>         getting the MAC address from a specific name.
>>
>>         Alex
>>
>>
>>         On 19 August 2014 11:30, Bala Manoharan
>>         <bala.manoharan@linaro.org <mailto:bala.manoharan@linaro.org>>
>>
>>         wrote:
>>
>>             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
>>             <alexandru.badicioiu@linaro.org
>>             <mailto:alexandru.badicioiu@linaro.org>> wrote:
>>
>>
>>
>>
>>                 On 19 August 2014 10:13, Bala Manoharan
>>                 <bala.manoharan@linaro.org
>>                 <mailto:bala.manoharan@linaro.org>> 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
>>                     <alexandru.badicioiu@linaro.org
>>                     <mailto:alexandru.badicioiu@linaro.org>> 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 <bala.manoharan@linaro.org
>>                         <mailto:bala.manoharan@linaro.org>> wrote:
>>
>>                             Regards,
>>                             Bala
>>                             Signed-off-by: Balasubramanian Manoharan
>>                             <bala.manoharan@linaro.org
>>                             <mailto:bala.manoharan@linaro.org>>
>>
>>                             ---
>>                              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
>>                             lng-odp@lists.linaro.org
>>                             <mailto: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
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

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;