All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Stanislav Fomichev <sdf@google.com>
Cc: asml.silence@gmail.com, axboe@kernel.dk,
	io-uring@vger.kernel.org, netdev@vger.kernel.org,
	davem@davemloft.net, kuba@kernel.org, edumazet@google.com,
	pabeni@redhat.com, linux-kernel@vger.kernel.org, leit@meta.com,
	bpf@vger.kernel.org, ast@kernel.org, martin.lau@linux.dev
Subject: Re: [PATCH 2/4] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT
Date: Fri, 28 Jul 2023 10:03:40 -0700	[thread overview]
Message-ID: <ZMP07KtOeJ09ejAd@gmail.com> (raw)
In-Reply-To: <ZMAAMKTaKSIKi1RW@google.com>

Hello Stanislav,

On Tue, Jul 25, 2023 at 10:02:40AM -0700, Stanislav Fomichev wrote:
> On 07/25, Breno Leitao wrote:
> > On Mon, Jul 24, 2023 at 10:31:28AM -0700, Stanislav Fomichev wrote:
> > > On 07/24, Breno Leitao wrote:
> > > > Add support for getsockopt command (SOCKET_URING_OP_GETSOCKOPT), where
> > > > level is SOL_SOCKET. This is leveraging the sockptr_t infrastructure,
> > > > where a sockptr_t is either userspace or kernel space, and handled as
> > > > such.
> > > > 
> > > > Function io_uring_cmd_getsockopt() is inspired by __sys_getsockopt().
> > > 
> > > We probably need to also have bpf bits in the new
> > > io_uring_cmd_getsockopt?
> > 
> > It might be interesting to have the BPF hook for this function as
> > well, but I would like to do it in a following patch, so, I can
> > experiment with it better, if that is OK.

I spent smoe time looking at the problem, and I understand we want to
call something as BPF_CGROUP_RUN_PROG_{G,S}ETSOCKOPT() into
io_uring_cmd_{g,s}etsockopt().

Per the previous conversation with Williem,
io_uring_cmd_{g,s}etsockopt() should use optval as a user pointer (void __user
*optval), and optlen as a kernel integer (it comes as from the io_uring
SQE), such as:

	void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
	int optlen = READ_ONCE(cmd->sqe->optlen);

Function BPF_CGROUP_RUN_PROG_GETSOCKOPT() calls
__cgroup_bpf_run_filter_getsockopt() which expects userpointer for
optlen and optval.

At the same time BPF_CGROUP_RUN_PROG_GETSOCKOPT_KERN() expects kernel
pointers for both optlen and optval.

In this current patchset, it has user pointer for optval and kernel value
for optlen. I.e., a third combination.  So, none of the functions would
work properly, and we probably do not want to create another function.

I am wondering if it is a good idea to move
__cgroup_bpf_run_filter_getsockopt() to use sockptr_t, so, it will be
able to adapt to any combination.

Any feedback is appreciate.
Thanks!

  parent reply	other threads:[~2023-07-28 17:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-24 14:22 [PATCH 0/3] io_uring: Initial support for {s,g}etsockopt commands Breno Leitao
2023-07-24 14:22 ` [PATCH 1/4] net: expose sock_use_custom_sol_socket Breno Leitao
2023-07-24 14:22 ` [PATCH 2/4] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT Breno Leitao
2023-07-24 17:31   ` Stanislav Fomichev
2023-07-25  9:27     ` Breno Leitao
2023-07-25 17:02       ` Stanislav Fomichev
2023-07-25 17:56         ` Martin KaFai Lau
2023-07-26  9:26           ` Breno Leitao
2023-07-28 17:03         ` Breno Leitao [this message]
2023-07-28 18:07           ` Stanislav Fomichev
2023-07-31 10:13             ` Breno Leitao
2023-07-24 22:58   ` Willem de Bruijn
2023-07-25  9:51     ` Breno Leitao
2023-07-25 13:56       ` Willem de Bruijn
2023-07-25 15:23         ` Breno Leitao
2023-07-24 14:22 ` [PATCH 3/4] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT Breno Leitao
2023-07-24 14:22 ` [PATCH 4/4] io_uring/cmd: Extend support beyond SOL_SOCKET Breno Leitao

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=ZMP07KtOeJ09ejAd@gmail.com \
    --to=leitao@debian.org \
    --cc=asml.silence@gmail.com \
    --cc=ast@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=io-uring@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=leit@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@google.com \
    /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.