All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Network Development <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	linux-arch@vger.kernel.org,
	y2038 Mailman List <y2038@lists.linaro.org>,
	Eric Dumazet <edumazet@google.com>,
	Willem de Bruijn <willemb@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-hams@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	linux-can@vger.kernel.org, dccp@vger.kernel.org,
	linux-wpan@vger.kernel.org, linux-sctp@vger.kernel.org,
	linux-x25@vger.kernel.org
Subject: Re: [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling
Date: Fri, 31 Aug 2018 11:08:09 -0400	[thread overview]
Message-ID: <CAF=yD-+tiVeP0FA1CUrT4Jyn28tnT+bett+Dq2frvU5eS1ANPw@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a00DLR9JyfKwbYXnKCpRf+YWuUyMbC+UfXZTfdnAJxRtg@mail.gmail.com>

> > > Looking at it again, it seems that sock_gettstamp() should
> > > actually deal with this gracefully: it will return a -EINVAL
> > > error condition if the timestamp remains at the
> > > SK_DEFAULT_STAMP initial value, which is probably
> > > just as appropriate (or better) as the current -ENOTTY
> > > default, and if we are actually recording timestamps, we
> > > might just as well report them.
> >
> > Yes, that's a nice solution. There is always some risk in changing
> > error codes. But ioctl callers should be able to support newly
> > implemented functionality. Even if partially implemented and
> > returning ENOENT instead of ENOIOCTLCMD.
>
> Ok, so do you think we should stay with the current version
> for now, and change the two points later, or should I rework
> it to integrate the locking and removing the callback?
>
> I suppose the series actually gets nicer without the
> callback, since I can simply add the generic timestamping
> implementation first, and then remove the dead ioctl
> handlers.

Agreed. I would add the locks in a separate patch, if only on the
off-chance that lockdep discovers something and it will be easier
to bisect and revise independently. I can also follow up with that
patch outside this set, of course.

WARNING: multiple messages have this Message-ID (diff)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Network Development <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	linux-arch@vger.kernel.org,
	y2038 Mailman List <y2038@lists.linaro.org>,
	Eric Dumazet <edumazet@google.com>,
	Willem de Bruijn <willemb@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-hams@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	linux-can@vger.kernel.org, dccp@vger.kernel.org,
	linux-wpan@vger.kernel.org, linux-sctp@vger.kernel.org,
	linux-x25@vger.kernel.org
Subject: Re: [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling
Date: Fri, 31 Aug 2018 15:08:09 +0000	[thread overview]
Message-ID: <CAF=yD-+tiVeP0FA1CUrT4Jyn28tnT+bett+Dq2frvU5eS1ANPw@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a00DLR9JyfKwbYXnKCpRf+YWuUyMbC+UfXZTfdnAJxRtg@mail.gmail.com>

> > > Looking at it again, it seems that sock_gettstamp() should
> > > actually deal with this gracefully: it will return a -EINVAL
> > > error condition if the timestamp remains at the
> > > SK_DEFAULT_STAMP initial value, which is probably
> > > just as appropriate (or better) as the current -ENOTTY
> > > default, and if we are actually recording timestamps, we
> > > might just as well report them.
> >
> > Yes, that's a nice solution. There is always some risk in changing
> > error codes. But ioctl callers should be able to support newly
> > implemented functionality. Even if partially implemented and
> > returning ENOENT instead of ENOIOCTLCMD.
>
> Ok, so do you think we should stay with the current version
> for now, and change the two points later, or should I rework
> it to integrate the locking and removing the callback?
>
> I suppose the series actually gets nicer without the
> callback, since I can simply add the generic timestamping
> implementation first, and then remove the dead ioctl
> handlers.

Agreed. I would add the locks in a separate patch, if only on the
off-chance that lockdep discovers something and it will be easier
to bisect and revise independently. I can also follow up with that
patch outside this set, of course.

WARNING: multiple messages have this Message-ID (diff)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: dccp@vger.kernel.org
Subject: Re: [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling
Date: Fri, 31 Aug 2018 15:08:09 +0000	[thread overview]
Message-ID: <CAF=yD-+tiVeP0FA1CUrT4Jyn28tnT+bett+Dq2frvU5eS1ANPw@mail.gmail.com> (raw)
In-Reply-To: <20180829130308.3504560-1-arnd@arndb.de>

> > > Looking at it again, it seems that sock_gettstamp() should
> > > actually deal with this gracefully: it will return a -EINVAL
> > > error condition if the timestamp remains at the
> > > SK_DEFAULT_STAMP initial value, which is probably
> > > just as appropriate (or better) as the current -ENOTTY
> > > default, and if we are actually recording timestamps, we
> > > might just as well report them.
> >
> > Yes, that's a nice solution. There is always some risk in changing
> > error codes. But ioctl callers should be able to support newly
> > implemented functionality. Even if partially implemented and
> > returning ENOENT instead of ENOIOCTLCMD.
>
> Ok, so do you think we should stay with the current version
> for now, and change the two points later, or should I rework
> it to integrate the locking and removing the callback?
>
> I suppose the series actually gets nicer without the
> callback, since I can simply add the generic timestamping
> implementation first, and then remove the dead ioctl
> handlers.

Agreed. I would add the locks in a separate patch, if only on the
off-chance that lockdep discovers something and it will be easier
to bisect and revise independently. I can also follow up with that
patch outside this set, of course.

  reply	other threads:[~2018-08-31 15:08 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29 12:59 [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling Arnd Bergmann
2018-08-29 12:59 ` Arnd Bergmann
2018-08-29 12:59 ` Arnd Bergmann
2018-08-29 12:59 ` [PATCH net-next 2/3] asm-generic: generalize asm/sockios.h Arnd Bergmann
2018-08-29 12:59   ` Arnd Bergmann
2018-08-29 13:11 ` [PATCH net-next 3/3] net: socket: implement 64-bit timestamps Arnd Bergmann
2018-08-29 13:11   ` Arnd Bergmann
2018-08-30 20:09 ` [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling Willem de Bruijn
2018-08-30 20:09   ` Willem de Bruijn
2018-08-30 20:09   ` Willem de Bruijn
2018-08-31 10:31   ` Arnd Bergmann
2018-08-31 10:31     ` Arnd Bergmann
2018-08-31 10:31     ` Arnd Bergmann
2018-08-31 13:37     ` Willem de Bruijn
2018-08-31 13:37       ` Willem de Bruijn
2018-08-31 13:37       ` Willem de Bruijn
2018-08-31 14:00       ` Arnd Bergmann
2018-08-31 14:00         ` Arnd Bergmann
2018-08-31 14:00         ` Arnd Bergmann
2018-08-31 15:08         ` Willem de Bruijn [this message]
2018-08-31 15:08           ` Willem de Bruijn
2018-08-31 15:08           ` Willem de Bruijn
2018-09-13 12:28 ` Arnd Bergmann
2018-09-13 12:28   ` Arnd Bergmann
2018-09-13 12:28   ` Arnd Bergmann
2018-09-21  9:14 ` Stefan Schmidt
2018-09-21  9:14   ` Stefan Schmidt
2018-09-21  9:14   ` Stefan Schmidt
2019-04-16 20:32 Arnd Bergmann
2019-04-16 20:32 ` Arnd Bergmann
2019-04-16 20:32 ` Arnd Bergmann
2019-04-16 20:32 ` Arnd Bergmann
2019-04-16 20:32 ` [PATCH net-next 2/3] asm-generic: generalize asm/sockios.h Arnd Bergmann
2019-04-16 20:32   ` Arnd Bergmann
2019-04-16 20:32 ` [PATCH net-next 3/3] net: socket: implement 64-bit timestamps Arnd Bergmann
2019-04-16 20:32   ` Arnd Bergmann
2019-04-17  9:35 ` [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling Neil Horman
2019-04-17  9:35   ` Neil Horman
2019-04-17  9:35   ` Neil Horman
2019-04-17  9:35   ` Neil Horman
2019-04-17  9:35   ` Neil Horman
2019-04-17 17:21   ` David Miller
2019-04-17 17:21     ` David Miller
2019-04-17 17:21     ` David Miller
2019-04-17 20:15     ` Neil Horman
2019-04-17 20:15       ` Neil Horman
2019-04-17 20:15       ` Neil Horman
2019-04-17  9:59 ` Marc Kleine-Budde
2019-04-17  9:59   ` Marc Kleine-Budde
2019-04-17  9:59   ` Marc Kleine-Budde
2019-04-17 14:46 ` Willem de Bruijn
2019-04-17 14:46   ` Willem de Bruijn
2019-04-17 14:46   ` Willem de Bruijn
2019-04-17 14:46   ` Willem de Bruijn
2019-04-17 14:46   ` Willem de Bruijn
2019-04-17 16:19   ` Arnd Bergmann
2019-04-17 16:19     ` Arnd Bergmann
2019-04-17 16:19     ` Arnd Bergmann
2019-04-17 16:19     ` Arnd Bergmann
2019-04-17 18:16     ` Willem de Bruijn
2019-04-17 18:16       ` Willem de Bruijn
2019-04-17 18:16       ` Willem de Bruijn
2019-04-17 18:16       ` 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='CAF=yD-+tiVeP0FA1CUrT4Jyn28tnT+bett+Dq2frvU5eS1ANPw@mail.gmail.com' \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=dccp@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-hams@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=linux-x25@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.