Worrying about the performance of functions like promiscuous enabling is misplaced since this routine would hardly be used in a performance path.
The real issue is the syntax. Has Petri defined what he wants for these yet? This seems like a simply attribute on the API odp_pktio_t object. Either it is or is not in promiscuous mode. As such, to be consistent with similar attributes one would expect the getter to be: odp_bool_t odp_pktio_promisc(odp_pktio_t io); and the setter to be: void odp_pktio_set_promisc(odp_pktio_t io, odp_bool_t truefalse); Bill On Thu, Nov 27, 2014 at 3:12 PM, Ola Liljedahl <[email protected]> wrote: > On 27 November 2014 at 18:51, Maxim Uvarov <[email protected]> > wrote: > > Define API and implement promisc functions for linux-generic. > > > > Signed-off-by: Maxim Uvarov <[email protected]> > > --- > > platform/linux-generic/include/api/odp_packet_io.h | 24 +++++++ > > platform/linux-generic/odp_packet_io.c | 74 > ++++++++++++++++++++++ > > 2 files changed, 98 insertions(+) > > > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h > b/platform/linux-generic/include/api/odp_packet_io.h > > index 667395c..c7d1e40 100644 > > --- a/platform/linux-generic/include/api/odp_packet_io.h > > +++ b/platform/linux-generic/include/api/odp_packet_io.h > > @@ -149,6 +149,30 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); > > int odp_pktio_mtu(odp_pktio_t id); > > > > /** > > + * Enable promiscuous mode on a packet IO interface. > > + * > > + * @param[in] id ODP packet IO handle. > > + * @param[in] enable 1 enabled, 0 disabled. > > + * > > + * @retval 0 on success. > > + * @retval -1 on a bad pktio id > > + * @retval -1 any other error > > + */ > > +int odp_pktio_promisc_set(odp_pktio_t id, odp_bool_t enable); > > + > > +/** > > + * Determine if promiscuous mode is enabled for a packet IO interface. > > + * > > + * @param[in] id ODP packet IO handle. > > + * > > + * @retval 1 if promiscuous mode is enabled. > > + * @retval 0 if promiscuous mode is disabled. > > + * @retval -1 on a bad pktio id > > + * @retval -1 any other error > > +*/ > > +int odp_pktio_promisc_enabled(odp_pktio_t id); > > + > > +/** > > * @} > > */ > > > > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > > index c523350..9140620 100644 > > --- a/platform/linux-generic/odp_packet_io.c > > +++ b/platform/linux-generic/odp_packet_io.c > > @@ -542,3 +542,77 @@ int odp_pktio_mtu(odp_pktio_t id) > > > > return ifr.ifr_mtu; > > } > > + > > +int odp_pktio_promisc_set(odp_pktio_t id, odp_bool_t enable) > > +{ > > + pktio_entry_t *entry; > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + entry = get_entry(id); > > + if (entry == NULL) { > > + ODP_DBG("pktio entry %d does not exist\n", id); > > + return -1; > > + } > > + > > + if (entry->s.pkt_sock_mmap.sockfd) > > + sockfd = entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > > + ifr.ifr_name[IFNAMSIZ] = 0; > > + > > + ret = ioctl(sockfd, SIOCGIFFLAGS, &ifr); > > + if (ret < 0) { > > + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); > > + return -1; > > + } > > + > > + if (enable) > > + ifr.ifr_flags |= IFF_PROMISC; > > + else > > + ifr.ifr_flags &= ~(IFF_PROMISC); > As some people are concerned with branching caused by different values > of the enable parameter, we can express this in a branch-less way: > #define UPDATE(old, mask, ena) ((old) & ~(mask)) | (((ena) ? ~0U : 0) & > (mask)) > ifr.ifr_flags = UPDATE(ifr.ifr_flags, IFF_PROMISC, enable); > > GCC for ARMv7 generates pretty compact code for this, no branches > (uses on ITE (if-then-else) instruction though). > > Branch-less code is preferred if the branches are difficult to predict > (e.g. more or less random). But don't overdo it since modern branch > predictors are quite good. > > > > + > > + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); > > + if (ret < 0) { > > + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +int odp_pktio_promisc_enabled(odp_pktio_t id) > > +{ > > + pktio_entry_t *entry; > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + entry = get_entry(id); > > + if (entry == NULL) { > > + ODP_DBG("pktio entry %d does not exist\n", id); > > + return -1; > > + } > > + > > + if (entry->s.pkt_sock_mmap.sockfd) > > + sockfd = entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > > + ifr.ifr_name[IFNAMSIZ] = 0; > > + > > + ret = ioctl(sockfd, SIOCGIFFLAGS, &ifr); > > + if (ret < 0) { > > + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); > > + return -1; > > + } > > + > > + if (ifr.ifr_flags & IFF_PROMISC) > > + return 1; > > + else > > + return 0; > > +} > > -- > > 1.8.5.1.163.gd7aced9 > > > > > > _______________________________________________ > > lng-odp mailing list > > [email protected] > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
