| Message ID | C1BBCB674C57E643932FC661E0B0D466239D0391@xmb-aln-x01.cisco.com |
|---|---|
| State | New |
| Headers | show |
Hi, There is no objection to the current API proposal, the discussions are concerning the addition of new API proposal from Alex. IMO, I will provide an updated patch incorporating some minor coding comments and we can check-in this patch as of now. Once we finalize on Alex and Petri's new proposal we can add that as an add-on patch. Does this make sense to everyone. P.S: IpSec application can query the mac address using pktio_t handle API which is provided in this patch. Regards, Bala On 20 August 2014 00:34, Robbie King (robking) <robking@cisco.com> wrote: > My apologies for not paying closer attention during the meeting today, > do we now have a plan of action to > > address querying the MAC address? That is currently a gating item for the > IPsec application patch. > > > > *From:* lng-odp-bounces@lists.linaro.org [mailto: > lng-odp-bounces@lists.linaro.org] *On Behalf Of *Savolainen, Petri (NSN - > FI/Espoo) > *Sent:* Tuesday, August 19, 2014 8:24 AM > *To:* ext Bala Manoharan; Alexandru Badicioiu > > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH 1/1] API support to get mac address of an > interface using odp_pktio_t > > > > Hi, > > > > As Bala mentioned, the intention is that user refers to an interface (by > name) only once and that’s in odp_pktio_open(). The resulting handle is > used in other packet IO calls after that. > > > > We may need to specify two different handle types / APIs: one for managing > interface level configuration (cf. physical function, one per interface), > and one for packet IO / configuration read access (cf. virtual function, > many per interface). The first would be used e.g. to set interface MAC > address or promiscuous mode. The second could be used to read current MAC > address, etc - in addition to packet IO. > > > > -Petri > > > > > > > > > > *From:* lng-odp-bounces@lists.linaro.org [ > mailto:lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>] > *On Behalf Of *ext Bala Manoharan > *Sent:* Tuesday, August 19, 2014 12:14 PM > *To:* Alexandru Badicioiu > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH 1/1] API support to get mac address of an > interface using odp_pktio_t > > > > 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(ðreq, 0, sizeof(ethreq)); > strncpy(ethreq.ifr_name, netdev, IFNAMSIZ); > - ret = ioctl(pkt_sock->sockfd, SIOCGIFHWADDR, ðreq); > + ret = ioctl(sockfd, SIOCGIFHWADDR, ðreq); > 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 > > > > > > > > > > > > >
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(ðreq, 0, sizeof(ethreq)); strncpy(ethreq.ifr_name, netdev, IFNAMSIZ); - ret = ioctl(pkt_sock->sockfd, SIOCGIFHWADDR, ðreq); + ret = ioctl(sockfd, SIOCGIFHWADDR, ðreq); 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;