Agreed, I don't believe it is necessary any more. -----Original Message----- From: Bala Manoharan [mailto:[email protected]] Sent: Wednesday, March 11, 2015 9:15 AM To: Ciprian Barbu Cc: Robbie King (robking); lng-odp; Ola Liljedahl; Anders Roxell; Maxim Uvarov; Taras Kondratiuk; Mike Holmes Subject: Re: [lng-odp] [PATCH] example: odp_ipsec: fix missing definition for ifreq
On 11 March 2015 at 18:31, Ciprian Barbu <[email protected]> wrote: > So after a little chat we had, Maxim suggested I remove the ioctl code > that needs struct ifreq. There is a USE_MAC_ADDR_HACK define in > odp_ipsec that is now set to 1, removing all that code should fix the > problem. > > The problem was described here: https://bugs.linaro.org/show_bug.cgi?id=627 > > Robbie and Bala, can you confirm that we can go ahead with this? Is > there any reason to keep that workaround? Hi, This hack was used before the API odp_pktio_mac_addr() was finalised. Since this API has been finalised now and the same has been added in odp_packet_io.h file. IMO this hack can be removed. Regards, Bala > > Thanks, > /Ciprian > > On Wed, Mar 11, 2015 at 2:45 PM, Ciprian Barbu <[email protected]> > wrote: >> On Wed, Mar 11, 2015 at 2:12 PM, Ola Liljedahl <[email protected]> >> wrote: >>> On 11 March 2015 at 10:58, Ciprian Barbu <[email protected]> wrote: >>>> >>>> On Tue, Mar 10, 2015 at 7:19 PM, Ola Liljedahl <[email protected]> >>>> wrote: >>>> > On 10 March 2015 at 17:11, Ciprian Barbu <[email protected]> >>>> > wrote: >>>> >> On Tue, Mar 10, 2015 at 6:06 PM, Ciprian Barbu >>>> >> <[email protected]> wrote: >>>> >>> On Tue, Mar 10, 2015 at 4:31 PM, Maxim Uvarov >>>> >>> <[email protected]> wrote: >>>> >>>> On 03/10/15 17:16, Ciprian Barbu wrote: >>>> >>>>> >>>> >>>>> On Tue, Mar 10, 2015 at 1:33 PM, Maxim Uvarov >>>> >>>>> <[email protected]> >>>> >>>>> wrote: >>>> >>>>>> >>>> >>>>>> Please also specify your env. I can not reproduce it with >>>> >>>>>> ./cross-compile-test.sh >>>> >>>>> >>>> >>>>> I added some info in the bug entry. Were you able to reproduce like >>>> >>>>> that? >>>> >>>> >>>> >>>> >>>> >>>> I see that in one includes in net/if.h that structure is under ifdef >>>> >>>> __USE_MISC, in other includes there is no such ifdef. >>>> >>>> Looks like you have different version of headers. For linux/if.h >>>> >>>> there is no >>>> >>>> ifdef for both cases. I think you patch is good to go, >>>> >>>> tested it on my toolchains (compilation only). >>>> >>> >>>> >>> This might be a problem with Ubuntu 13.10, I tested on an Ubuntu 14.04 >>>> >>> and it works. >>>> >>> >>>> >>> The whole problem comes from the ioctl command that requires struct >>>> >>> ifreq. From this man page (http://linux.die.net/man/7/netdevice) it >>>> >>> looks like it should be enough to include <sys/ioctl.h> and >>>> >>> <net/if.h>. I also found that including linux/if.h is usually done by >>>> >>> code for kernel, so that might actually not be a good idea. >>>> >>> >>>> >>> Strange though, adding <sys/ioctl.h> doesn't fix compiling on my >>>> >>> environment. Does anyone else run Ubuntu 13.10? maybe I screwed my >>>> >>> headers somehow installing some packages ... >>>> >> >>>> >> I also found this: >>>> >> >>>> >> http://stackoverflow.com/questions/10433982/why-does-c99-complain-about-storage-sizes >>>> >> which says the problem is in fact with -std=c99. Strange though how it >>>> >> only behaves bad on my Ubuntu 13.10. Would still be good if someone >>>> >> else checks on their env ... >>>> > The way I interpret this is that when you specify a specific C >>>> > standard (e.g. C99), by default you will only get access to those >>>> > library features that are included in that standard. If you need to go >>>> > outside of the standard, you need to ask for it specifically. E.g. >>>> > define _BSD_SOURCE in this case. >>>> > >>>> > But this should be independent of Ubuntu releases. I can imagine >>>> > different libc (glibc) versions may do this differently though so this >>>> > could be that cause of differences in behavior between 13.10 and e.g. >>>> > 14.04 (which I use). >>>> >>>> Ola, we used to make use of _BSD_SOURCE to get the extra features, but >>>> the use of this macro has been deprecated since glib 2.20: >>>> http://man7.org/linux/man-pages/man7/feature_test_macros.7.html. >>>> Instead of defining _BSD_SOURCE the user must rely on _DEFAULT_SOURCE, >>>> which exists since 2.19. >>>> >>>> Looking at my glib version I get: >>>> GNU C Library (Ubuntu EGLIBC 2.17-93ubuntu4) stable release version >>>> 2.17, by Roland McGrath et al. >>>> >>>> So the problem in my case is that I have an old glibc version and the >>>> way we compile ODP does not cope with that. Other users will be >>>> affected. Either we do something about it (I haven't found a way to >>>> check the glibc version at compile time yet) or document that ODP will >>>> not support glibc older than 2.19 (where _DEFAULT_SOURCE exists). >>> >>> Or we add some code to automake/configure to detect which (g)libc version >>> you are using and then use the appropriate define (_BSD_SOURCE for <= 2.17, >>> _DEFAULT_SOURCE for >= 2.19, don't know about 2.18). >>> >>> Wouldn't this be the "right" way of solving problems like this? This might >>> not be the only time we have such a version problem. >> >> Sounds good. I think autotools should have the support to get the >> version of packages. >> >> Anders, do you have more insights on that? >> >>> >>> -- Ola >>> >>>> >>>> > >>>> > olli@macmini:~$ gcc --version >>>> > gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2 >>>> > Copyright (C) 2013 Free Software Foundation, Inc. >>>> > This is free software; see the source for copying conditions. There is >>>> > NO >>>> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR >>>> > PURPOSE. >>>> > olli@macmini:~$ dpkg -S /usr/include/net/if.h >>>> > libc6-dev:amd64, libc6-dev:i386: /usr/include/net/if.h >>>> > olli@macmini:~$ dpkg -s libc6-dev | grep Version >>>> > Version: 2.19-0ubuntu6.6 >>>> > >>>> > >>>> >> >>>> >> /Ciprian >>>> >> >>>> >>> >>>> >>>> >>>> >>>> Maxim. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>>>> Maxim. >>>> >>>>>> >>>> >>>>>> >>>> >>>>>> On 03/10/15 14:14, Maxim Uvarov wrote: >>>> >>>>>>> >>>> >>>>>>> Please add patch description and put bug link in the bottom of it. >>>> >>>>>>> Like >>>> >>>>>>> other git commits do. >>>> >>>>>>> >>>> >>>>>>> Thanks, >>>> >>>>>>> Maxim. >>>> >>>>>>> >>>> >>>>>>> On 03/10/15 12:47, Ciprian Barbu wrote: >>>> >>>>>>>> >>>> >>>>>>>> Signed-off-by: Ciprian Barbu <[email protected]> >>>> >>>>>>>> --- >>>> >>>>>>>> fix for https://bugs.linaro.org/show_bug.cgi?id=1330 >>>> >>>>>>>> >>>> >>>>>>>> example/ipsec/odp_ipsec.c | 2 +- >>>> >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>>>>>> >>>> >>>>>>>> diff --git a/example/ipsec/odp_ipsec.c >>>> >>>>>>>> b/example/ipsec/odp_ipsec.c >>>> >>>>>>>> index 98160ba..286b9f0 100644 >>>> >>>>>>>> --- a/example/ipsec/odp_ipsec.c >>>> >>>>>>>> +++ b/example/ipsec/odp_ipsec.c >>>> >>>>>>>> @@ -30,7 +30,7 @@ >>>> >>>>>>>> #include <stdbool.h> >>>> >>>>>>>> #include <sys/socket.h> >>>> >>>>>>>> -#include <net/if.h> >>>> >>>>>>>> +#include <linux/if.h> >>>> >>>>>>>> #include <sys/ioctl.h> >>>> >>>>>>>> #include <sys/socket.h> >>>> >>>>>>> >>>> >>>>>>> >>>> >>>>>> >>>> >>>>>> _______________________________________________ >>>> >>>>>> 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
