| Message ID | 1417796424-9909-4-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 5 December 2014 at 17:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Define API to get MAC address for specific packet i/o and > implement linux-generic version. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ > platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h > index 5df1b13..236f201 100644 > --- a/platform/linux-generic/include/api/odp_packet_io.h > +++ b/platform/linux-generic/include/api/odp_packet_io.h > @@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); > int odp_pktio_promisc_mode(odp_pktio_t id); > > /** > + * Get the default MAC address of a packet IO interface. > + * > + * @param[in] id ODP packet IO handle. Why not call this parameter "handle" or "hdl"? "id" is supposedly an abbreviation for "identifier" but is also a proper word in its own right. If using "id" as an abbreviation, the parameter description should be "ODP packet IO identifier". > + * @param[out] mac_addr Storage for MAC address of the packet IO interface. > + * @param[in] addr_size Storage size for the address I think the doxygen [in][out] rules were changed last week. In this example, only mac_addr need to have the [out] tag. > + * > + * @retval Number of bytes written on success, 0 on failure. > + */ > +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, > + size_t addr_size); > + > +/** > * @} > */ > > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index 7d2d2a4..e8815cd 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -21,6 +21,7 @@ > > #include <string.h> > #include <sys/ioctl.h> > +#include <linux/if_arp.h> > > typedef struct { > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > @@ -633,3 +634,40 @@ int odp_pktio_promisc_mode(odp_pktio_t id) > else > return 0; > } > + > +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, > + size_t addr_size) > +{ > + pktio_entry_t *entry; > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (addr_size < ETH_ALEN) > + return 0; > + > + entry = get_entry(id); > + if (entry == NULL) { > + ODP_DBG("pktio entry %d does not exist\n", id); > + return 0; > + } > + > + lock_entry(entry); > + > + sockfd = sockfd_from_pktio_entry(entry); > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > + ifr.ifr_name[IFNAMSIZ - 1] = 0; > + > + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); > + if (ret < 0) { > + unlock_entry(entry); > + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); > + return 0; > + } > + > + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, > + ETH_ALEN); > + unlock_entry(entry); > + > + return ETH_ALEN; > +} > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 12/08/2014 02:13 PM, Ola Liljedahl wrote: > On 5 December 2014 at 17:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> Define API to get MAC address for specific packet i/o and >> implement linux-generic version. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ >> platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ >> 2 files changed, 50 insertions(+) >> >> diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h >> index 5df1b13..236f201 100644 >> --- a/platform/linux-generic/include/api/odp_packet_io.h >> +++ b/platform/linux-generic/include/api/odp_packet_io.h >> @@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); >> int odp_pktio_promisc_mode(odp_pktio_t id); >> >> /** >> + * Get the default MAC address of a packet IO interface. >> + * >> + * @param[in] id ODP packet IO handle. > Why not call this parameter "handle" or "hdl"? "id" is supposedly an > abbreviation for "identifier" but is also a proper word in its own > right. > If using "id" as an abbreviation, the parameter description should be > "ODP packet IO identifier". Everywhere it's id. I plan to send separate patch for renaming id to hdl. >> + * @param[out] mac_addr Storage for MAC address of the packet IO interface. >> + * @param[in] addr_size Storage size for the address > I think the doxygen [in][out] rules were changed last week. > In this example, only mac_addr need to have the [out] tag. ok. >> + * >> + * @retval Number of bytes written on success, 0 on failure. >> + */ >> +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, >> + size_t addr_size); >> + >> +/** >> * @} >> */ >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index 7d2d2a4..e8815cd 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -21,6 +21,7 @@ >> >> #include <string.h> >> #include <sys/ioctl.h> >> +#include <linux/if_arp.h> >> >> typedef struct { >> pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; >> @@ -633,3 +634,40 @@ int odp_pktio_promisc_mode(odp_pktio_t id) >> else >> return 0; >> } >> + >> +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, >> + size_t addr_size) >> +{ >> + pktio_entry_t *entry; >> + int sockfd; >> + struct ifreq ifr; >> + int ret; >> + >> + if (addr_size < ETH_ALEN) >> + return 0; >> + >> + entry = get_entry(id); >> + if (entry == NULL) { >> + ODP_DBG("pktio entry %d does not exist\n", id); >> + return 0; >> + } >> + >> + lock_entry(entry); >> + >> + sockfd = sockfd_from_pktio_entry(entry); >> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >> + >> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >> + if (ret < 0) { >> + unlock_entry(entry); >> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >> + return 0; >> + } >> + >> + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >> + ETH_ALEN); >> + unlock_entry(entry); >> + >> + return ETH_ALEN; >> +} >> -- >> 1.8.5.1.163.gd7aced9 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
On 2014-12-08 16:25, Maxim Uvarov wrote: > On 12/08/2014 02:13 PM, Ola Liljedahl wrote: > >On 5 December 2014 at 17:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > >>Define API to get MAC address for specific packet i/o and > >>implement linux-generic version. > >> > >>Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > >>--- > >> platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ > >> platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ > >> 2 files changed, 50 insertions(+) > >> > >>diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h > >>index 5df1b13..236f201 100644 > >>--- a/platform/linux-generic/include/api/odp_packet_io.h > >>+++ b/platform/linux-generic/include/api/odp_packet_io.h > >>@@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); > >> int odp_pktio_promisc_mode(odp_pktio_t id); > >> > >> /** > >>+ * Get the default MAC address of a packet IO interface. > >>+ * > >>+ * @param[in] id ODP packet IO handle. > >Why not call this parameter "handle" or "hdl"? "id" is supposedly an > >abbreviation for "identifier" but is also a proper word in its own > >right. > >If using "id" as an abbreviation, the parameter description should be > >"ODP packet IO identifier". > Everywhere it's id. I plan to send separate patch for renaming id to hdl. Now there has been two that got confused that you named the variable to "id" and not "handle" or "hdl". do the cleanup patch first and then add this patch. You can just do the cleanup patch before in this patchset. Cheers, Anders
On 12/09/2014 12:38 PM, Anders Roxell wrote: > On 2014-12-08 16:25, Maxim Uvarov wrote: >> On 12/08/2014 02:13 PM, Ola Liljedahl wrote: >>> On 5 December 2014 at 17:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>>> Define API to get MAC address for specific packet i/o and >>>> implement linux-generic version. >>>> >>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>>> --- >>>> platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ >>>> platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ >>>> 2 files changed, 50 insertions(+) >>>> >>>> diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h >>>> index 5df1b13..236f201 100644 >>>> --- a/platform/linux-generic/include/api/odp_packet_io.h >>>> +++ b/platform/linux-generic/include/api/odp_packet_io.h >>>> @@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); >>>> int odp_pktio_promisc_mode(odp_pktio_t id); >>>> >>>> /** >>>> + * Get the default MAC address of a packet IO interface. >>>> + * >>>> + * @param[in] id ODP packet IO handle. >>> Why not call this parameter "handle" or "hdl"? "id" is supposedly an >>> abbreviation for "identifier" but is also a proper word in its own >>> right. >>> If using "id" as an abbreviation, the parameter description should be >>> "ODP packet IO identifier". >> Everywhere it's id. I plan to send separate patch for renaming id to hdl. > Now there has been two that got confused that you named the variable to > "id" and not "handle" or "hdl". > do the cleanup patch first and then add this patch. > You can just do the cleanup patch before in this patchset. > > Cheers, > Anders I can do it after as well. It's not confusing because everywhere in the code it is odp_pktio_t id. Patch should not depend on renaming anyhow. Maxim.
On 5 December 2014 at 08:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Define API to get MAC address for specific packet i/o and > implement linux-generic version. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ > platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h > index 5df1b13..236f201 100644 > --- a/platform/linux-generic/include/api/odp_packet_io.h > +++ b/platform/linux-generic/include/api/odp_packet_io.h > @@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); > int odp_pktio_promisc_mode(odp_pktio_t id); > > /** > + * Get the default MAC address of a packet IO interface. > + * > + * @param[in] id ODP packet IO handle. > + * @param[out] mac_addr Storage for MAC address of the packet IO interface. > + * @param[in] addr_size Storage size for the address > + * > + * @retval Number of bytes written on success, 0 on failure. > + */ > +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, > + size_t addr_size); > + > +/** > * @} > */ > > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index 7d2d2a4..e8815cd 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -21,6 +21,7 @@ > > #include <string.h> > #include <sys/ioctl.h> > +#include <linux/if_arp.h> > > typedef struct { > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > @@ -633,3 +634,40 @@ int odp_pktio_promisc_mode(odp_pktio_t id) > else > return 0; > } > + > +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, > + size_t addr_size) > +{ > + pktio_entry_t *entry; > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (addr_size < ETH_ALEN) > + return 0; > + > + entry = get_entry(id); > + if (entry == NULL) { > + ODP_DBG("pktio entry %d does not exist\n", id); > + return 0; > + } > + > + lock_entry(entry); Here you need to check that entry is not free, before doing anything with it. Look at what odp_pktio_recv does. Note get_entry does not care whether entry is free or not. But even if it would do that between get_entry and lock_entry there is a gap where it could be closed. The same goes for other places like this. Thanks, Victor > + > + sockfd = sockfd_from_pktio_entry(entry); > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > + ifr.ifr_name[IFNAMSIZ - 1] = 0; > + > + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); > + if (ret < 0) { > + unlock_entry(entry); > + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); > + return 0; > + } > + > + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, > + ETH_ALEN); > + unlock_entry(entry); > + > + return ETH_ALEN; > +} > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h index 5df1b13..236f201 100644 --- a/platform/linux-generic/include/api/odp_packet_io.h +++ b/platform/linux-generic/include/api/odp_packet_io.h @@ -175,6 +175,18 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id, odp_bool_t enable); int odp_pktio_promisc_mode(odp_pktio_t id); /** + * Get the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[out] mac_addr Storage for MAC address of the packet IO interface. + * @param[in] addr_size Storage size for the address + * + * @retval Number of bytes written on success, 0 on failure. + */ +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, + size_t addr_size); + +/** * @} */ diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 7d2d2a4..e8815cd 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -21,6 +21,7 @@ #include <string.h> #include <sys/ioctl.h> +#include <linux/if_arp.h> typedef struct { pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; @@ -633,3 +634,40 @@ int odp_pktio_promisc_mode(odp_pktio_t id) else return 0; } + +size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, + size_t addr_size) +{ + pktio_entry_t *entry; + int sockfd; + struct ifreq ifr; + int ret; + + if (addr_size < ETH_ALEN) + return 0; + + entry = get_entry(id); + if (entry == NULL) { + ODP_DBG("pktio entry %d does not exist\n", id); + return 0; + } + + lock_entry(entry); + + sockfd = sockfd_from_pktio_entry(entry); + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); + ifr.ifr_name[IFNAMSIZ - 1] = 0; + + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); + if (ret < 0) { + unlock_entry(entry); + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); + return 0; + } + + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, + ETH_ALEN); + unlock_entry(entry); + + return ETH_ALEN; +}
Define API to get MAC address for specific packet i/o and implement linux-generic version. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/include/api/odp_packet_io.h | 12 +++++++ platform/linux-generic/odp_packet_io.c | 38 ++++++++++++++++++++++ 2 files changed, 50 insertions(+)