All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: dccp@vger.kernel.org
Subject: Re: [PATCH net-next v3] net: ioctl: Use kernel memory on protocol ioctl callbacks
Date: Fri, 26 May 2023 08:49:10 +0000	[thread overview]
Message-ID: <ZHByhjt2xYf5xKAu@gmail.com> (raw)
In-Reply-To: <20230525125503.400797-1-leitao@debian.org>

On Thu, May 25, 2023 at 11:05:40AM -0400, Willem de Bruijn wrote:
> On Thu, May 25, 2023 at 8:55 AM Breno Leitao <leitao@debian.org> wrote:
> > @@ -1547,6 +1547,28 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
> >         return ret;
> >  }
> >
> > +/* Execute if this ioctl is a special mroute ioctl */
> > +int ipmr_sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
> > +{
> > +       switch (cmd) {
> > +       /* These userspace buffers will be consumed by ipmr_ioctl() */
> > +       case SIOCGETVIFCNT: {
> > +               struct sioc_vif_req buffer;
> > +
> > +               return sock_ioctl_inout(sk, cmd, arg, &buffer,
> > +                                     sizeof(buffer));
> > +               }
> 
> More importantly, if we go down the path of demultiplexing in protocol
> independent code to call protocol specific handlers, then there there
> is no need to have them call protocol independent helpers like
> sock_ioct_inout again. Just call the protocol-specific ioctl handlers
> directly?

That is what I was expecting, but, the code is exactly the same and I
kept it in the generic section. This is what this code needs to do:

 * Copy X byte from userspace
 * sk->sk_prot->ioctl()
 * Copy X bytes back to userspace

I tried to keep the code generic enough that it could be reused.
I can definitely push the same code to two different protocols, if you
prefer, no strong opinion. 

WARNING: multiple messages have this Message-ID (diff)
From: Breno Leitao <leitao@debian.org>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: dsahern@kernel.org, Remi Denis-Courmont <courmisch@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Alexander Aring <alex.aring@gmail.com>,
	Stefan Schmidt <stefan@datenfreihafen.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Matthieu Baerts <matthieu.baerts@tessares.net>,
	Mat Martineau <martineau@kernel.org>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	Xin Long <lucien.xin@gmail.com>,
	leit@fb.com, axboe@kernel.dk, asml.silence@gmail.com,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	dccp@vger.kernel.org, linux-wpan@vger.kernel.org,
	mptcp@lists.linux.dev, linux-sctp@vger.kernel.org
Subject: Re: [PATCH net-next v3] net: ioctl: Use kernel memory on protocol ioctl callbacks
Date: Fri, 26 May 2023 01:49:10 -0700	[thread overview]
Message-ID: <ZHByhjt2xYf5xKAu@gmail.com> (raw)
In-Reply-To: <CAF=yD-LHQNkgPb-R==53-2auVxkP9r=xqrz2A8oe61vkoDdWjg@mail.gmail.com>

On Thu, May 25, 2023 at 11:05:40AM -0400, Willem de Bruijn wrote:
> On Thu, May 25, 2023 at 8:55 AM Breno Leitao <leitao@debian.org> wrote:
> > @@ -1547,6 +1547,28 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
> >         return ret;
> >  }
> >
> > +/* Execute if this ioctl is a special mroute ioctl */
> > +int ipmr_sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
> > +{
> > +       switch (cmd) {
> > +       /* These userspace buffers will be consumed by ipmr_ioctl() */
> > +       case SIOCGETVIFCNT: {
> > +               struct sioc_vif_req buffer;
> > +
> > +               return sock_ioctl_inout(sk, cmd, arg, &buffer,
> > +                                     sizeof(buffer));
> > +               }
> 
> More importantly, if we go down the path of demultiplexing in protocol
> independent code to call protocol specific handlers, then there there
> is no need to have them call protocol independent helpers like
> sock_ioct_inout again. Just call the protocol-specific ioctl handlers
> directly?

That is what I was expecting, but, the code is exactly the same and I
kept it in the generic section. This is what this code needs to do:

 * Copy X byte from userspace
 * sk->sk_prot->ioctl()
 * Copy X bytes back to userspace

I tried to keep the code generic enough that it could be reused.
I can definitely push the same code to two different protocols, if you
prefer, no strong opinion. 

  parent reply	other threads:[~2023-05-26  8:49 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-25 12:54 [PATCH net-next v3] net: ioctl: Use kernel memory on protocol ioctl callbacks Breno Leitao
2023-05-25 12:54 ` Breno Leitao
2023-05-25 14:19 ` Eric Dumazet
2023-05-25 14:19   ` Eric Dumazet
2023-05-25 14:54 ` Simon Horman
2023-05-25 14:54   ` Simon Horman
2023-05-25 15:03 ` David Ahern
2023-05-25 15:03   ` David Ahern
2023-05-25 15:05 ` Willem de Bruijn
2023-05-25 15:05   ` Willem de Bruijn
2023-05-25 15:16 ` net: ioctl: Use kernel memory on protocol ioctl callbacks: Tests Results MPTCP CI
2023-05-25 15:34 ` [PATCH net-next v3] net: ioctl: Use kernel memory on protocol ioctl callbacks David Ahern
2023-05-25 15:34   ` David Ahern
2023-05-25 16:06 ` Willem de Bruijn
2023-05-25 16:06   ` Willem de Bruijn
2023-05-26  1:15 ` kernel test robot
2023-05-26  8:49 ` Breno Leitao [this message]
2023-05-26  8:49   ` Breno Leitao
2023-05-26  9:08 ` Breno Leitao
2023-05-26  9:08   ` Breno Leitao
2023-05-26 14:09 ` Willem de Bruijn
2023-05-26 14:09   ` Willem de Bruijn
2023-05-28 18:05 ` kernel test robot
2023-05-28 18:05   ` kernel test robot

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=ZHByhjt2xYf5xKAu@gmail.com \
    --to=leitao@debian.org \
    --cc=dccp@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.