Exactly.

I would expect ODP implementations to define MAX_MAC_ADDRESS to 8 because
that will cover MAC addresses up to EUI-64. Haven't seen or heard of any
MAC addresses larger than 64 bits.


On 4 September 2014 17:25, Bala Manoharan <[email protected]> wrote:

> Hi Ola,
>
> Since odp_pktio_t is an abstract type shouldn't the size of the mac
> address (bytes) be returned by the implementation rather than the
> application. The Application can give a byte array of size MAX_MAC_ADDRESS
> and the implementation can return the size of the mac address of which
> belongs to this current interface.
>
> Regards,
> Bala
>
>
> On 4 September 2014 19:50, Ola Liljedahl <[email protected]> wrote:
>
>> I would still use a byte array, not a 64-bit integer. The application
>> specifies the size of the byte array (buffer) being passed in the call, the
>> function will check whether the buffer is large enough to hold the MAC
>> address. This would replace the current implicit assumption that the byte
>> array is (at least) six bytes large.
>>
>>
>>
>> On 4 September 2014 15:57, Savolainen, Petri (NSN - FI/Espoo) <
>> [email protected]> wrote:
>>
>>>  If uint64_t would be used for mac, a set of functions would be needed
>>> to operate on it (e.g. copy it into a packet header, or check the bcast
>>> bit). Then a odp_eth_mac_t type would be better choice to the public API
>>> (instead of helpers).
>>>
>>>
>>>
>>> -Petri
>>>
>>>
>>>
>>>
>>>
>>> *From:* [email protected] [mailto:
>>> [email protected]] *On Behalf Of *ext Ola Liljedahl
>>> *Sent:* Thursday, September 04, 2014 4:01 PM
>>> *To:* Bala Manoharan; [email protected]
>>> *Subject:* Re: [lng-odp] [PATCH v3 1/1] API support for querying mac
>>> address
>>>
>>>
>>>
>>> I think the prototype should include the size of the buffer as well.
>>> Other types of network interfaces use 64-bit MAC addresses. The cost of
>>> looking forward is negligible.
>>>
>>>
>>>
>>> /**
>>>  * Get MAC address of the interface
>>>  *
>>>  * @param hdl          ODP packet IO handle
>>>  * @param mac_addr     Storage for MAC address of the packet IO
>>> interface (filled by function)
>>>
>>>  * @param buf_size     Size of mac_addr buffer.
>>>
>>>
>>>
>>>  * @return  Number of bytes written on success or -1 on error
>>> **/
>>> int odp_pktio_get_mac_addr(odp_pktio_t hdl, unsigned char *mac_addr,
>>> size_t buf_size);
>>>
>>>
>>>
>>>
>>>
>>> On 4 September 2014 14:51, Bala Manoharan <[email protected]>
>>> wrote:
>>>
>>> FYI
>>>
>>>
>>>
>>> ---------- Forwarded message ----------
>>> From: *Balasubramanian Manoharan* <[email protected]>
>>> Date: 20 August 2014 15:50
>>> Subject: [lng-odp] [PATCH v3 1/1] API support for querying mac address
>>> To: [email protected]
>>> Cc: Balasubramanian Manoharan <[email protected]>
>>>
>>>
>>> This patch provides API support for querying mac address of an interface
>>> using odp_pktio_t handle.
>>> This current patch incorporates the review comments from the previous
>>> patch.
>>>
>>> The discussions are ongoing regarding adding additional API for querying
>>> mac address using device name, once it gets finalized the same will be
>>> provided as a different patch.
>>>
>>> Signed-off-by: Balasubramanian Manoharan <[email protected]>
>>> ---
>>>  include/odp_packet_io.h                            |  8 ++++++
>>>  platform/linux-generic/include/odp_packet_netmap.h |  1 +
>>>  platform/linux-generic/odp_packet_io.c             | 29
>>> +++++++++++++++++++++-
>>>  platform/linux-generic/odp_packet_netmap.c         |  3 ++-
>>>  platform/linux-generic/odp_packet_socket.c         | 19 +++++++-------
>>>  5 files changed, 49 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/odp_packet_io.h b/include/odp_packet_io.h
>>> index cfefac0..86778bf 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..69174fe 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..ddad7a4 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,30 @@ 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) {
>>> +               ODP_ERR("Invalid odp_pktio_t value\n");
>>> +               return -1;
>>> +       }
>>> +       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..13f3f52 100644
>>> --- a/platform/linux-generic/odp_packet_netmap.c
>>> +++ b/platform/linux-generic/odp_packet_netmap.c
>>> @@ -222,9 +222,10 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> const char *netdev,
>>>                 ODP_ERR("Error: token creation failed\n");
>>>                 return -1;
>>>         }
>>> +       if (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..593d093 100644
>>> --- a/platform/linux-generic/odp_packet_socket.c
>>> +++ b/platform/linux-generic/odp_packet_socket.c
>>> @@ -57,6 +57,8 @@ typedef struct {
>>>
>>>  static raw_socket_t raw_sockets[MAX_RAW_SOCKETS_NETDEVS];
>>>  static odp_spinlock_t raw_sockets_lock;
>>> +static int socket_store_hw_addr(int sockfd, unsigned char *if_mac,
>>> +                             const char *netdev);
>>>
>>>  /** Eth buffer start offset from u32-aligned address to make sure the
>>> following
>>>   * header (e.g. IP) starts at a 32-bit aligned address.
>>> @@ -153,7 +155,6 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock,
>>> const char *netdev,
>>>         if (pool == ODP_BUFFER_POOL_INVALID)
>>>                 return -1;
>>>         pkt_sock->pool = pool;
>>> -
>>>         pkt = odp_packet_alloc(pool);
>>>         if (!odp_packet_is_valid(pkt))
>>>                 return -1;
>>> @@ -168,12 +169,14 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock,
>>> const char *netdev,
>>>         pkt_sock->max_frame_len = pkt_sock->buf_size -
>>> pkt_sock->frame_offset;
>>>
>>>         odp_packet_free(pkt);
>>> -
>>>         odp_spinlock_lock(&raw_sockets_lock);
>>> -
>>>         sockfd = find_raw_fd(netdev);
>>>         if (sockfd) {
>>>                 pkt_sock->sockfd = sockfd;
>>> +               if (socket_store_hw_addr(sockfd, pkt_sock->if_mac,
>>> netdev)) {
>>> +                       perror("setup_pkt_sock() -
>>> socket_store_hw_addr()");
>>> +                       goto error;
>>> +               }
>>>                 odp_spinlock_unlock(&raw_sockets_lock);
>>>                 return sockfd;
>>>         }
>>> @@ -215,7 +218,6 @@ int setup_pkt_sock(pkt_sock_t * const pkt_sock,
>>> const char *netdev,
>>>                 perror("setup_pkt_sock() - bind(to IF)");
>>>                 goto error;
>>>         }
>>> -
>>>         add_raw_fd(netdev, sockfd);
>>>         odp_spinlock_unlock(&raw_sockets_lock);
>>>         return sockfd;
>>> @@ -735,7 +737,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 +746,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,8 +807,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);
>>> -       if (ret != 0)
>>> +       if (socket_store_hw_addr(pkt_sock->sockfd, pkt_sock->if_mac,
>>> netdev))
>>>                 return -1;
>>>
>>>         if_idx = if_nametoindex(netdev);
>>> --
>>> 2.0.1.472.g6f92e5f
>>>
>>>
>>>
>>>
>>>
>>
>>
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to