[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CABeXuvomXSgYQaFQgv71xQu837J_HUYR4NvMqXPyh4Z3Btnprg@mail.gmail.com>
Date: Tue, 12 Feb 2019 11:08:52 -0800
From: Deepa Dinamani <deepa.kernel@...il.com>
To: Ran Rozenstein <ranro@...lanox.com>
Cc: "davem@...emloft.net" <davem@...emloft.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"arnd@...db.de" <arnd@...db.de>,
"y2038@...ts.linaro.org" <y2038@...ts.linaro.org>,
"chris@...kel.net" <chris@...kel.net>,
"fenghua.yu@...el.com" <fenghua.yu@...el.com>,
"rth@...ddle.net" <rth@...ddle.net>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"ubraun@...ux.ibm.com" <ubraun@...ux.ibm.com>,
"linux-alpha@...r.kernel.org" <linux-alpha@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>,
"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
"linux-xtensa@...ux-xtensa.org" <linux-xtensa@...ux-xtensa.org>,
"sparclinux@...r.kernel.org" <sparclinux@...r.kernel.org>
Subject: Re: [PATCH net-next v5 09/12] socket: Add SO_TIMESTAMPING_NEW
On Sun, Feb 10, 2019 at 7:21 PM Deepa Dinamani <deepa.kernel@...il.com> wrote:
>
> On Feb 10, 2019, at 7:43 AM, Ran Rozenstein <ranro@...lanox.com> wrote:
>
> >> Subject: [PATCH net-next v5 09/12] socket: Add SO_TIMESTAMPING_NEW
> >>
> >> Add SO_TIMESTAMPING_NEW variant of socket timestamp options.
> >> This is the y2038 safe versions of the SO_TIMESTAMPING_OLD for all
> >> architectures.
> >>
> >> Signed-off-by: Deepa Dinamani <deepa.kernel@...il.com>
> >> Acked-by: Willem de Bruijn <willemb@...gle.com>
> >
> >
> > Hi,
> >
> > I have app that include:
> > #include <linux/errqueue.h>
> >
> > It now fail with this error:
> > In file included from timestamping.c:6:0:
> > /usr/include/linux/errqueue.h:46:27: error: array type has incomplete element type 'struct __kernel_timespec'
> > struct __kernel_timespec ts[3];
> > ^~
> > I tried to do the trivial fix, to include time.h:
> > In include/uapi/linux/errqueue.h
> > #include <linux/time.h>
> > #include <linux/types.h>
> >
> > But it just add some other noises:
> > In file included from /usr/include/linux/errqueue.h:5:0,
> > from timestamping.c:6:
> > /usr/include/linux/time.h:10:8: error: redefinition of ?struct timespec?
> > struct timespec {
> > ^~~~~~~~
> > In file included from /usr/include/sys/select.h:39:0,
> > from /usr/include/sys/types.h:197,
> > from /usr/include/stdlib.h:279,
> > from timestamping.c:2:
> > /usr/include/bits/types/struct_timespec.h:8:8: note: originally defined here
> > struct timespec
> > ^~~~~~~~
> > In file included from /usr/include/linux/errqueue.h:5:0,
> > from timestamping.c:6:
> > /usr/include/linux/time.h:16:8: error: redefinition of ?struct timeval?
> > struct timeval {
> > ^~~~~~~
> > In file included from /usr/include/sys/select.h:37:0,
> > from /usr/include/sys/types.h:197,
> > from /usr/include/stdlib.h:279,
> > from timestamping.c:2:
> > /usr/include/bits/types/struct_timeval.h:8:8: note: originally defined here
> > struct timeval
> > ^~~~~~~
> >
> >
> > Can you please advise how to solve it?
> >
> > Thanks,
> > Ran
>
> The errqueue.h already had the same issue reported previously:
> https://lore.kernel.org/netdev/CAF=yD-L2ntuH54J_SwN9WcpBMgkV_v0e-Q2Pu2mrQ3+1RozGFQ@mail.gmail.com/
>
> Earlier when I tested this with kernel selftests such as
> tools/testing/selftests/networking/timestamping/rxtimestamp(the test
> was broken to begin with because of missing include of unistd.h), I
> was using make.cross to build.
> This does not put the headers in the right place
> (obj-$ARCH/usr/include instead of usr/include). Hence, I did not
> realize that this breaks the inclusion of errqueue.h due to the
> missing __kernel_timespec definition.
> I forgot that nobody seems to be using linux/time.h.
>
> But, if I include guards( #ifndef __KERNEL__) for struct timespec,
> struct timeval etc for linux/time.h, then we can include it from
> userspace/ errqueue.h for __kernel_timespec:
>
> --- a/include/uapi/linux/errqueue.h
> +++ b/include/uapi/linux/errqueue.h
> @@ -2,7 +2,7 @@
> #ifndef _UAPI_LINUX_ERRQUEUE_H
> #define _UAPI_LINUX_ERRQUEUE_H
>
> -#include <linux/types.h>
> +#include <linux/time.h>
>
> struct sock_extended_err {
> __u32 ee_errno;
> diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> index a6aca9aaab80..40913d9a5bc8 100644
> --- a/include/uapi/linux/time.h
> +++ b/include/uapi/linux/time.h
> @@ -5,6 +5,8 @@
> #include <linux/types.h>
>
>
> +#ifdef __KERNEL__
> +
> #ifndef _STRUCT_TIMESPEC
> #define _STRUCT_TIMESPEC
> struct timespec {
> @@ -42,6 +44,8 @@ struct itimerval {
> struct timeval it_value; /* current value */
> };
>
> +#endif /* __KERNEL__ */
>
> Arnd,
>
> I forgot how we plan to include the definition for __kernel_timespec
> for libc or userspace. Does this seem right to you?
> Also these changes to errqueue.h needs to be reverted probably as this
> breaks userspace.
Arnd and I talked about this today morning.
We agreed that we could introduce a new time_types.h along the lines
of posix_types.h. We will move all the time definitions that we plan
to keep in the kernel uapi headers to this header. This header will
also not have any overlap with the sys/time.h and can be included
along with it from userspace.
I will post this patch shortly.
This should fix Ran's issue.
-Deepa
Powered by blists - more mailing lists