All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: netdev@vger.kernel.org, David Miller <davem@davemloft.net>
Subject: Re: [RFC] separate SIOCGIFCONF from the rest of dev_ioctl()
Date: Mon, 26 Jun 2017 23:06:40 +0100	[thread overview]
Message-ID: <20170626220640.GC10672@ZenIV.linux.org.uk> (raw)
In-Reply-To: <1498508714.24675.5.camel@sipsolutions.net>

On Mon, Jun 26, 2017 at 10:25:14PM +0200, Johannes Berg wrote:
> On Mon, 2017-06-26 at 18:40 +0100, Al Viro wrote:
> 
> > 	Only two of dev_ioctl() callers may pass SIOCGIFCONF to it.
> > Separating that codepath from the rest of dev_ioctl() allows both
> > to simplify dev_ioctl() itself (all other cases work with struct
> > ifreq *)
> > *and* seriously simplify the compat side of that beast: all it takes
> > is passing to inet_gifconf() an extra argument - the size of
> > individual
> > records (sizeof(struct ifreq) or sizeof(struct compat_ifreq)).  With
> > dev_ifconf() called directly from sock_do_ioctl()/compat_dev_ifconf()
> > that's easy to arrange.
> 
> No objection from me; however, I just introduced another special case
> (in a bugfix for a >20yo bug ...) here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=68dd02d19c811ca8ea60220a9d73e13b4bdad73a
> 
> It would perhaps make sense to also pull that out into the caller,
> which could also get rid of the stupid way the #ifdef is placed in
> sock_ioctl(). For compat, it's already pulled out anyway, even a level
> up than where you're calling it for SIOCGIFCONF - might make sense to
> put the wext stuff into compat_sock_ioctl_trans() too.

BTW, speaking of odd ioctls - e.g. SIOCGIFMETRIC does
	* for AF_AX25, AF_X25, AF_IRDA, AF_NETROM, AF_ROSE: fail with -EINVAL
	* everything else - store 0 in ifr->ifr_metric and succeed.
Is there anything special about that set of families or is it just a cut'n'paste
accident?  SIOCGIFNETMASK:
	* AF_AX25, AF_X25, AF_IRDA, AF_NETROM, AF_ROSE, AF_IPX, AF_QIPCRTR: -EINVAL
	* AF_INET: return ipv4 netmask associated with the interface in question.
	* AF_PACKET: if CONFIG_INET => as AF_INET, otherwise -ENOIOCTLCMD
	* everything else: -ENOTTY, as far as I can tell
Again, what makes ax25 different from e.g. decnet?  netmask makes no sense for
either, and AFAICS in exact same way.  And the set of -EINVAL ones is similar,
but not identical to SIOCGIFMETRIC case...

And then there's SIOCGSTAMP and friends; AFAICS, they are identical for everything
that implements them, modulo differences in locking.  The set of those who have
those also looks fairly random...

      reply	other threads:[~2017-06-26 22:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-26 17:40 [RFC] separate SIOCGIFCONF from the rest of dev_ioctl() Al Viro
2017-06-26 20:25 ` Johannes Berg
2017-06-26 22:06   ` Al Viro [this message]

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=20170626220640.GC10672@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davem@davemloft.net \
    --cc=johannes@sipsolutions.net \
    --cc=netdev@vger.kernel.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.