linux-parisc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Deepa Dinamani <deepa.kernel@gmail.com>
To: willemdebruijn.kernel@gmail.com
Cc: "David S. Miller" <davem@davemloft.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Network Devel Mailing List <netdev@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	"James E.J. Bottomley" <jejb@parisc-linux.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	rth@twiddle.net, linux-alpha@vger.kernel.org,
	"open list:RALINK MIPS ARCHITECTURE" <linux-mips@linux-mips.org>,
	Parisc List <linux-parisc@vger.kernel.org>,
	linux-rdma@vger.kernel.org,
	sparclinux <sparclinux@vger.kernel.org>
Subject: Re: [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW
Date: Sat, 24 Nov 2018 21:55:50 -0800	[thread overview]
Message-ID: <CABeXuvr6fi9UPwMvuafx7MwEta-Sx1TLNN0KQ8HQHhn=vAne5Q@mail.gmail.com> (raw)
In-Reply-To: <CABeXuvoDPvT45CCt2L0TReqtb1Q_-E8+toYu6qjiP0cxfdQNdA@mail.gmail.com>

A couple of comments I missed:

> > > >  /*
> > > >   * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> > > >   * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> > > > @@ -719,19 +751,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> > > >                 false_tstamp = 1;
> > > >         }
> > > > -       if (need_software_tstamp) {
> > >
> > > Considerably less code churn if adding __sock_recv_timestamp_2038 and
> > > calling that here:
> > >
> > >                    if (sock_flag(sk, SOCK_TSTAMP_NEW))
> > >                            __sock_recv_timestamp_2038(msg, sk, skb);
> > >                    else if ...
> > >
> > > Same for the tcp case above, really, and in the case of the next patch
> > > for SO_TIMESTAMPING_NEW
> >
> > That naming convention, ..._2038, is not the nicest, of course. That
> > is not the relevant bit in the above comment.

it could be  __sock_recv_timestamp64().
But, these timestamps should be doing exactly the same thing as the
old ones and I thought it would be nicer to keep the same code path.
I can change it to as per above.

> > Come to think of it, and related to my question in patch 2 why the
> > need to rename at all, could all new structs, constants and functions
> > be named consistently with 64 suffix? __sock_recv_timestamp64,
> > SO_TIMESTAMPING64 and timeval64 (instead of sock_timeval,
> > it isn't really a sock specific struct)?
> >
> > I guess that there is a good reason for the renaming exercise and
> > conditional mapping of SO_TIMESTAMP onto old or new interface.
> > Please elucidate in the commit message.
>
> I think there is some confusion here.
>
> The existing timestamp options: SO_TIMESTAMP* fail to provide proper
> timestamps beyond year 2038 on 32 bit ABIs.
> But, these work fine on 64 bit native ABIs.
> So now we need a way of updating these timestamps so that we do not
> break existing userspace: 64 bit ABIs should not have to change
> userspace, 32 bit ABIs should work as is until 2038 after which they
> have bad timestamps.
> So we introduce new y2038 safe timestamp options for 32 bit ABIs. We
> assume that 32 bit applications will switch to new ABIs at some point,
> but leave the older timestamps as is.
> I can update the commit text as per above.

We have been avoiding adding timeval64 timestamps to discourage users
from using these types in the interfaces.
We want to keep all the uapi time interfaces to use __kernel_*
interfaces. And, we already provide __kernel_timespec interface for
such instances.
But, in this case we do not have an option. So we introduce a type
specific to sockets.

-Deepa

  reply	other threads:[~2018-11-25  5:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-24  2:20 [PATCH 0/8] net: y2038-safe socket timestamps Deepa Dinamani
2018-11-24  2:20 ` [PATCH 2/8] sockopt: Rename SO_TIMESTAMP* to SO_TIMESTAMP*_OLD Deepa Dinamani
2018-11-25  3:58   ` Willem de Bruijn
2018-11-30 22:38     ` Deepa Dinamani
2018-11-30 23:33       ` Willem de Bruijn
2018-11-24  2:20 ` [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW Deepa Dinamani
2018-11-25  3:59   ` Willem de Bruijn
2018-11-25  4:17     ` Willem de Bruijn
2018-11-25  5:28       ` Deepa Dinamani
2018-11-25  5:55         ` Deepa Dinamani [this message]
2018-11-25 14:38           ` Willem de Bruijn
2018-11-25 14:33         ` Willem de Bruijn
2018-11-25 22:35           ` Arnd Bergmann
2018-11-26  0:25             ` Willem de Bruijn
2018-11-30 22:43           ` Deepa Dinamani
2018-11-30 23:37             ` Willem de Bruijn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABeXuvr6fi9UPwMvuafx7MwEta-Sx1TLNN0KQ8HQHhn=vAne5Q@mail.gmail.com' \
    --to=deepa.kernel@gmail.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=jejb@parisc-linux.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=rth@twiddle.net \
    --cc=sparclinux@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=y2038@lists.linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).