| Message ID | 1418050713-29694-4-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Define API and implement promisc functions for linux-generic. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > platform/linux-generic/include/api/odp_packet_io.h | 22 ++++++ > platform/linux-generic/odp_packet_io.c | 84 ++++++++++++++++++++++ > 2 files changed, 106 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h > index 5f7ba7c..742ea59 100644 > --- a/platform/linux-generic/include/api/odp_packet_io.h > +++ b/platform/linux-generic/include/api/odp_packet_io.h > @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); > int odp_pktio_mtu(odp_pktio_t id); > > /** > + * Enable/Disable promiscuous mode on a packet IO interface. > + * > + * @param[in] id ODP packet IO handle. > + * @param[in] enable 1 to enable, 0 to disable. > + * > + * @retval 0 on success. > + * @retval non-zero on any error. > + */ > +int odp_pktio_promisc_mode_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 any error. > +*/ > +int odp_pktio_promisc_mode(odp_pktio_t id); > + > +/** > * @} > */ > > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index faa197f..7d2d2a4 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num) > return nbr; > } > > +/** function should be called with locked entry */ > +static int sockfd_from_pktio_entry(pktio_entry_t *entry) > +{ > + if (entry->s.pkt_sock_mmap.sockfd >= 0) I don't agree with this approach, you should first look at the pktio type as set by odp_pktio_open and get the socket depending on that. Some sanity checks might be needed too. > + return entry->s.pkt_sock_mmap.sockfd; > + else > + return entry->s.pkt_sock.sockfd; > +} > + > int odp_pktio_set_mtu(odp_pktio_t id, int mtu) > { > pktio_entry_t *entry; > @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) > > return ifr.ifr_mtu; > } > + > +int odp_pktio_promisc_mode_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; > + } > + > + 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, SIOCGIFFLAGS, &ifr); > + if (ret < 0) { > + unlock_entry(entry); > + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); > + return -1; > + } > + > + if (enable) > + ifr.ifr_flags |= IFF_PROMISC; > + else > + ifr.ifr_flags &= ~(IFF_PROMISC); > + > + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); > + if (ret < 0) { > + unlock_entry(entry); > + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); > + return -1; > + } > + > + unlock_entry(entry); > + return 0; > +} > + > +int odp_pktio_promisc_mode(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; > + } > + > + 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, SIOCGIFFLAGS, &ifr); > + if (ret < 0) { > + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); > + unlock_entry(entry); > + return -1; > + } > + unlock_entry(entry); > + > + if (ifr.ifr_flags & IFF_PROMISC) > + return 1; > + else > + return 0; > +} > -- > 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/09/2014 06:18 PM, Ciprian Barbu wrote: > On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> Define API and implement promisc functions for linux-generic. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> >> --- >> platform/linux-generic/include/api/odp_packet_io.h | 22 ++++++ >> platform/linux-generic/odp_packet_io.c | 84 ++++++++++++++++++++++ >> 2 files changed, 106 insertions(+) >> >> diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h >> index 5f7ba7c..742ea59 100644 >> --- a/platform/linux-generic/include/api/odp_packet_io.h >> +++ b/platform/linux-generic/include/api/odp_packet_io.h >> @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); >> int odp_pktio_mtu(odp_pktio_t id); >> >> /** >> + * Enable/Disable promiscuous mode on a packet IO interface. >> + * >> + * @param[in] id ODP packet IO handle. >> + * @param[in] enable 1 to enable, 0 to disable. >> + * >> + * @retval 0 on success. >> + * @retval non-zero on any error. >> + */ >> +int odp_pktio_promisc_mode_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 any error. >> +*/ >> +int odp_pktio_promisc_mode(odp_pktio_t id); >> + >> +/** >> * @} >> */ >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index faa197f..7d2d2a4 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num) >> return nbr; >> } >> >> +/** function should be called with locked entry */ >> +static int sockfd_from_pktio_entry(pktio_entry_t *entry) >> +{ >> + if (entry->s.pkt_sock_mmap.sockfd >= 0) > I don't agree with this approach, you should first look at the pktio > type as set by odp_pktio_open and get the socket depending on that. > Some sanity checks might be needed too. For what? -1 is not initialized socket. >= 0 socket is in use. Why should I do code more complex? Maxim. > >> + return entry->s.pkt_sock_mmap.sockfd; >> + else >> + return entry->s.pkt_sock.sockfd; >> +} >> + >> int odp_pktio_set_mtu(odp_pktio_t id, int mtu) >> { >> pktio_entry_t *entry; >> @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) >> >> return ifr.ifr_mtu; >> } >> + >> +int odp_pktio_promisc_mode_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; >> + } >> + >> + 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, SIOCGIFFLAGS, &ifr); >> + if (ret < 0) { >> + unlock_entry(entry); >> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >> + return -1; >> + } >> + >> + if (enable) >> + ifr.ifr_flags |= IFF_PROMISC; >> + else >> + ifr.ifr_flags &= ~(IFF_PROMISC); >> + >> + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); >> + if (ret < 0) { >> + unlock_entry(entry); >> + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); >> + return -1; >> + } >> + >> + unlock_entry(entry); >> + return 0; >> +} >> + >> +int odp_pktio_promisc_mode(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; >> + } >> + >> + 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, SIOCGIFFLAGS, &ifr); >> + if (ret < 0) { >> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >> + unlock_entry(entry); >> + return -1; >> + } >> + unlock_entry(entry); >> + >> + if (ifr.ifr_flags & IFF_PROMISC) >> + return 1; >> + else >> + return 0; >> +} >> -- >> 1.8.5.1.163.gd7aced9 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
On Tue, Dec 9, 2014 at 5:31 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/09/2014 06:18 PM, Ciprian Barbu wrote: >> >> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >> wrote: >>> >>> Define API and implement promisc functions for linux-generic. >>> >>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> >>> --- >>> platform/linux-generic/include/api/odp_packet_io.h | 22 ++++++ >>> platform/linux-generic/odp_packet_io.c | 84 >>> ++++++++++++++++++++++ >>> 2 files changed, 106 insertions(+) >>> >>> diff --git a/platform/linux-generic/include/api/odp_packet_io.h >>> b/platform/linux-generic/include/api/odp_packet_io.h >>> index 5f7ba7c..742ea59 100644 >>> --- a/platform/linux-generic/include/api/odp_packet_io.h >>> +++ b/platform/linux-generic/include/api/odp_packet_io.h >>> @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); >>> int odp_pktio_mtu(odp_pktio_t id); >>> >>> /** >>> + * Enable/Disable promiscuous mode on a packet IO interface. >>> + * >>> + * @param[in] id ODP packet IO handle. >>> + * @param[in] enable 1 to enable, 0 to disable. >>> + * >>> + * @retval 0 on success. >>> + * @retval non-zero on any error. >>> + */ >>> +int odp_pktio_promisc_mode_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 any error. >>> +*/ >>> +int odp_pktio_promisc_mode(odp_pktio_t id); >>> + >>> +/** >>> * @} >>> */ >>> >>> diff --git a/platform/linux-generic/odp_packet_io.c >>> b/platform/linux-generic/odp_packet_io.c >>> index faa197f..7d2d2a4 100644 >>> --- a/platform/linux-generic/odp_packet_io.c >>> +++ b/platform/linux-generic/odp_packet_io.c >>> @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, >>> odp_buffer_hdr_t *buf_hdr[], int num) >>> return nbr; >>> } >>> >>> +/** function should be called with locked entry */ >>> +static int sockfd_from_pktio_entry(pktio_entry_t *entry) >>> +{ >>> + if (entry->s.pkt_sock_mmap.sockfd >= 0) >> >> I don't agree with this approach, you should first look at the pktio >> type as set by odp_pktio_open and get the socket depending on that. >> Some sanity checks might be needed too. > > > For what? -1 is not initialized socket. >= 0 socket is in use. Why should I > do code more complex? What if the socket handles get corrupted? You're basing your selection between pkt_sock_mmap and pkt_sock on the assumption that one of them will be initialized. Either we factor out the socket descriptor or we look in the right structure based on the pktio type. One other example, someone might call this internal function after the pktio has been freed. > > Maxim. > > >> >>> + return entry->s.pkt_sock_mmap.sockfd; >>> + else >>> + return entry->s.pkt_sock.sockfd; >>> +} >>> + >>> int odp_pktio_set_mtu(odp_pktio_t id, int mtu) >>> { >>> pktio_entry_t *entry; >>> @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) >>> >>> return ifr.ifr_mtu; >>> } >>> + >>> +int odp_pktio_promisc_mode_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; >>> + } >>> + >>> + 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, SIOCGIFFLAGS, &ifr); >>> + if (ret < 0) { >>> + unlock_entry(entry); >>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>> + return -1; >>> + } >>> + >>> + if (enable) >>> + ifr.ifr_flags |= IFF_PROMISC; >>> + else >>> + ifr.ifr_flags &= ~(IFF_PROMISC); >>> + >>> + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); >>> + if (ret < 0) { >>> + unlock_entry(entry); >>> + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); >>> + return -1; >>> + } >>> + >>> + unlock_entry(entry); >>> + return 0; >>> +} >>> + >>> +int odp_pktio_promisc_mode(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; >>> + } >>> + >>> + 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, SIOCGIFFLAGS, &ifr); >>> + if (ret < 0) { >>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>> + unlock_entry(entry); >>> + return -1; >>> + } >>> + unlock_entry(entry); >>> + >>> + if (ifr.ifr_flags & IFF_PROMISC) >>> + return 1; >>> + else >>> + return 0; >>> +} >>> -- >>> 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/09/2014 06:38 PM, Ciprian Barbu wrote: > On Tue, Dec 9, 2014 at 5:31 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 12/09/2014 06:18 PM, Ciprian Barbu wrote: >>> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >>> wrote: >>>> Define API and implement promisc functions for linux-generic. >>>> >>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>>> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> >>>> --- >>>> platform/linux-generic/include/api/odp_packet_io.h | 22 ++++++ >>>> platform/linux-generic/odp_packet_io.c | 84 >>>> ++++++++++++++++++++++ >>>> 2 files changed, 106 insertions(+) >>>> >>>> diff --git a/platform/linux-generic/include/api/odp_packet_io.h >>>> b/platform/linux-generic/include/api/odp_packet_io.h >>>> index 5f7ba7c..742ea59 100644 >>>> --- a/platform/linux-generic/include/api/odp_packet_io.h >>>> +++ b/platform/linux-generic/include/api/odp_packet_io.h >>>> @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); >>>> int odp_pktio_mtu(odp_pktio_t id); >>>> >>>> /** >>>> + * Enable/Disable promiscuous mode on a packet IO interface. >>>> + * >>>> + * @param[in] id ODP packet IO handle. >>>> + * @param[in] enable 1 to enable, 0 to disable. >>>> + * >>>> + * @retval 0 on success. >>>> + * @retval non-zero on any error. >>>> + */ >>>> +int odp_pktio_promisc_mode_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 any error. >>>> +*/ >>>> +int odp_pktio_promisc_mode(odp_pktio_t id); >>>> + >>>> +/** >>>> * @} >>>> */ >>>> >>>> diff --git a/platform/linux-generic/odp_packet_io.c >>>> b/platform/linux-generic/odp_packet_io.c >>>> index faa197f..7d2d2a4 100644 >>>> --- a/platform/linux-generic/odp_packet_io.c >>>> +++ b/platform/linux-generic/odp_packet_io.c >>>> @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, >>>> odp_buffer_hdr_t *buf_hdr[], int num) >>>> return nbr; >>>> } >>>> >>>> +/** function should be called with locked entry */ >>>> +static int sockfd_from_pktio_entry(pktio_entry_t *entry) >>>> +{ >>>> + if (entry->s.pkt_sock_mmap.sockfd >= 0) >>> I don't agree with this approach, you should first look at the pktio >>> type as set by odp_pktio_open and get the socket depending on that. >>> Some sanity checks might be needed too. >> >> For what? -1 is not initialized socket. >= 0 socket is in use. Why should I >> do code more complex? > What if the socket handles get corrupted? You're basing your selection > between pkt_sock_mmap and pkt_sock on the assumption that one of them > will be initialized. Either we factor out the socket descriptor or we > look in the right structure based on the pktio type. > > One other example, someone might call this internal function after the > pktio has been freed. if clear: In that case you will call ioctl on -1 and error will be returned. Does not matter which socket is -1. in corrupted case it's bug. But also error will be returned. Nothing bad. Maxim. >> Maxim. >> >> >>>> + return entry->s.pkt_sock_mmap.sockfd; >>>> + else >>>> + return entry->s.pkt_sock.sockfd; >>>> +} >>>> + >>>> int odp_pktio_set_mtu(odp_pktio_t id, int mtu) >>>> { >>>> pktio_entry_t *entry; >>>> @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) >>>> >>>> return ifr.ifr_mtu; >>>> } >>>> + >>>> +int odp_pktio_promisc_mode_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; >>>> + } >>>> + >>>> + 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, SIOCGIFFLAGS, &ifr); >>>> + if (ret < 0) { >>>> + unlock_entry(entry); >>>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>>> + return -1; >>>> + } >>>> + >>>> + if (enable) >>>> + ifr.ifr_flags |= IFF_PROMISC; >>>> + else >>>> + ifr.ifr_flags &= ~(IFF_PROMISC); >>>> + >>>> + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); >>>> + if (ret < 0) { >>>> + unlock_entry(entry); >>>> + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); >>>> + return -1; >>>> + } >>>> + >>>> + unlock_entry(entry); >>>> + return 0; >>>> +} >>>> + >>>> +int odp_pktio_promisc_mode(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; >>>> + } >>>> + >>>> + 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, SIOCGIFFLAGS, &ifr); >>>> + if (ret < 0) { >>>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>>> + unlock_entry(entry); >>>> + return -1; >>>> + } >>>> + unlock_entry(entry); >>>> + >>>> + if (ifr.ifr_flags & IFF_PROMISC) >>>> + return 1; >>>> + else >>>> + return 0; >>>> +} >>>> -- >>>> 1.8.5.1.163.gd7aced9 >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>
Note my reply on similar code path. See below. On 9 December 2014 at 07:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/09/2014 06:38 PM, Ciprian Barbu wrote: >> >> On Tue, Dec 9, 2014 at 5:31 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >> wrote: >>> >>> On 12/09/2014 06:18 PM, Ciprian Barbu wrote: >>>> >>>> On Mon, Dec 8, 2014 at 4:58 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >>>> wrote: >>>>> >>>>> Define API and implement promisc functions for linux-generic. >>>>> >>>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>>>> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> >>>>> --- >>>>> platform/linux-generic/include/api/odp_packet_io.h | 22 ++++++ >>>>> platform/linux-generic/odp_packet_io.c | 84 >>>>> ++++++++++++++++++++++ >>>>> 2 files changed, 106 insertions(+) >>>>> >>>>> diff --git a/platform/linux-generic/include/api/odp_packet_io.h >>>>> b/platform/linux-generic/include/api/odp_packet_io.h >>>>> index 5f7ba7c..742ea59 100644 >>>>> --- a/platform/linux-generic/include/api/odp_packet_io.h >>>>> +++ b/platform/linux-generic/include/api/odp_packet_io.h >>>>> @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); >>>>> int odp_pktio_mtu(odp_pktio_t id); >>>>> >>>>> /** >>>>> + * Enable/Disable promiscuous mode on a packet IO interface. >>>>> + * >>>>> + * @param[in] id ODP packet IO handle. >>>>> + * @param[in] enable 1 to enable, 0 to disable. >>>>> + * >>>>> + * @retval 0 on success. >>>>> + * @retval non-zero on any error. >>>>> + */ >>>>> +int odp_pktio_promisc_mode_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 any error. >>>>> +*/ >>>>> +int odp_pktio_promisc_mode(odp_pktio_t id); >>>>> + >>>>> +/** >>>>> * @} >>>>> */ >>>>> >>>>> diff --git a/platform/linux-generic/odp_packet_io.c >>>>> b/platform/linux-generic/odp_packet_io.c >>>>> index faa197f..7d2d2a4 100644 >>>>> --- a/platform/linux-generic/odp_packet_io.c >>>>> +++ b/platform/linux-generic/odp_packet_io.c >>>>> @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, >>>>> odp_buffer_hdr_t *buf_hdr[], int num) >>>>> return nbr; >>>>> } >>>>> >>>>> +/** function should be called with locked entry */ >>>>> +static int sockfd_from_pktio_entry(pktio_entry_t *entry) >>>>> +{ >>>>> + if (entry->s.pkt_sock_mmap.sockfd >= 0) >>>> >>>> I don't agree with this approach, you should first look at the pktio >>>> type as set by odp_pktio_open and get the socket depending on that. >>>> Some sanity checks might be needed too. >>> >>> >>> For what? -1 is not initialized socket. >= 0 socket is in use. Why should >>> I >>> do code more complex? >> >> What if the socket handles get corrupted? You're basing your selection >> between pkt_sock_mmap and pkt_sock on the assumption that one of them >> will be initialized. Either we factor out the socket descriptor or we >> look in the right structure based on the pktio type. >> >> One other example, someone might call this internal function after the >> pktio has been freed. > > > if clear: > In that case you will call ioctl on -1 and error will be returned. Does not > matter which socket > is -1. > > in corrupted case it's bug. But also error will be returned. Nothing bad. > > Maxim. > > > >>> Maxim. >>> >>> >>>>> + return entry->s.pkt_sock_mmap.sockfd; >>>>> + else >>>>> + return entry->s.pkt_sock.sockfd; >>>>> +} >>>>> + >>>>> int odp_pktio_set_mtu(odp_pktio_t id, int mtu) >>>>> { >>>>> pktio_entry_t *entry; >>>>> @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) >>>>> >>>>> return ifr.ifr_mtu; >>>>> } >>>>> + >>>>> +int odp_pktio_promisc_mode_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; >>>>> + } >>>>> + >>>>> + lock_entry(entry); You need to check whether entry is not freed at this point. The same as pktio_rcv do after lock_entry. That, I guess, would address Ciprian concern that you maybe working on entry that got closed already. If entry is not closed, I agree one of sockets should be valid, and if someone overwrites them it is a bug. 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, SIOCGIFFLAGS, &ifr); >>>>> + if (ret < 0) { >>>>> + unlock_entry(entry); >>>>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>>>> + return -1; >>>>> + } >>>>> + >>>>> + if (enable) >>>>> + ifr.ifr_flags |= IFF_PROMISC; >>>>> + else >>>>> + ifr.ifr_flags &= ~(IFF_PROMISC); >>>>> + >>>>> + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); >>>>> + if (ret < 0) { >>>>> + unlock_entry(entry); >>>>> + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); >>>>> + return -1; >>>>> + } >>>>> + >>>>> + unlock_entry(entry); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int odp_pktio_promisc_mode(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; >>>>> + } >>>>> + >>>>> + 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, SIOCGIFFLAGS, &ifr); >>>>> + if (ret < 0) { >>>>> + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); >>>>> + unlock_entry(entry); >>>>> + return -1; >>>>> + } >>>>> + unlock_entry(entry); >>>>> + >>>>> + if (ifr.ifr_flags & IFF_PROMISC) >>>>> + return 1; >>>>> + else >>>>> + return 0; >>>>> +} >>>>> -- >>>>> 1.8.5.1.163.gd7aced9 >>>>> >>>>> >>>>> _______________________________________________ >>>>> 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 --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h index 5f7ba7c..742ea59 100644 --- a/platform/linux-generic/include/api/odp_packet_io.h +++ b/platform/linux-generic/include/api/odp_packet_io.h @@ -153,6 +153,28 @@ int odp_pktio_set_mtu(odp_pktio_t id, int mtu); int odp_pktio_mtu(odp_pktio_t id); /** + * Enable/Disable promiscuous mode on a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[in] enable 1 to enable, 0 to disable. + * + * @retval 0 on success. + * @retval non-zero on any error. + */ +int odp_pktio_promisc_mode_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 any error. +*/ +int odp_pktio_promisc_mode(odp_pktio_t id); + +/** * @} */ diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index faa197f..7d2d2a4 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -486,6 +486,15 @@ int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num) return nbr; } +/** function should be called with locked entry */ +static int sockfd_from_pktio_entry(pktio_entry_t *entry) +{ + if (entry->s.pkt_sock_mmap.sockfd >= 0) + return entry->s.pkt_sock_mmap.sockfd; + else + return entry->s.pkt_sock.sockfd; +} + int odp_pktio_set_mtu(odp_pktio_t id, int mtu) { pktio_entry_t *entry; @@ -549,3 +558,78 @@ int odp_pktio_mtu(odp_pktio_t id) return ifr.ifr_mtu; } + +int odp_pktio_promisc_mode_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; + } + + 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, SIOCGIFFLAGS, &ifr); + if (ret < 0) { + unlock_entry(entry); + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); + return -1; + } + + if (enable) + ifr.ifr_flags |= IFF_PROMISC; + else + ifr.ifr_flags &= ~(IFF_PROMISC); + + ret = ioctl(sockfd, SIOCSIFFLAGS, &ifr); + if (ret < 0) { + unlock_entry(entry); + ODP_DBG("ioctl SIOCSIFFLAGS error\n"); + return -1; + } + + unlock_entry(entry); + return 0; +} + +int odp_pktio_promisc_mode(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; + } + + 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, SIOCGIFFLAGS, &ifr); + if (ret < 0) { + ODP_DBG("ioctl SIOCGIFFLAGS error\n"); + unlock_entry(entry); + return -1; + } + unlock_entry(entry); + + if (ifr.ifr_flags & IFF_PROMISC) + return 1; + else + return 0; +}