Linux Documentation
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Daniel Gomez <da.gomez@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-block@vger.kernel.org, linux-doc@vger.kernel.org,
	bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
	Luis Chamberlain <mcgrof@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Brendan Gregg <brendan.d.gregg@gmail.com>,
	GOST <gost.dev@samsung.com>,
	Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Subject: Re: configurable block error injection
Date: Fri, 5 Jun 2026 09:23:38 +0200	[thread overview]
Message-ID: <20260605072338.GA26929@lst.de> (raw)
In-Reply-To: <d9a5b911-7692-4df2-b67c-c33bc80b61aa@kernel.org>

On Thu, Jun 04, 2026 at 01:06:13PM +0200, Daniel Gomez wrote:
> IIU the series correctly, and oversimplifying: when
> injection is enabled and a bio matches, the block layer completes the
> bio inline with the chosen blk_status_t (the status= rule from debugfs)
> via bio_endio_status(). The submission path returns to the caller
> immediately, with the bio already in the error state. Nothing is ever
> sent to the device, but the completion path sees the injected error.

Yes.

> > I'd have to allow access to certain bio
> 
> With struct_ops the bio is passed to the ebpf side as read-only, bio 
> fields can be read to decide the policy but cannot write them. Is 
> read-only access to bio fields itself a concern?

Yes, for the reason state below.

> 
> > fields and would have create a stable UAPI for commands and status
> > using the fake BTF struct access which really would not be a good
> > idea here as we need to be able to change internals.  
> 
> That should not be a problem at all. With CO-RE (compile once, run
> everywhere) the program resolves the bio field offsets against the
> BTF of the kernel it loads on, so it adapts dynamically if the layout
> changes. The contract is just the struct_ops callback signature: a
> struct bio * argument and a blk_status_t return. And that doesn't imply
> any UAPI commitment AFAIK.

That assumes the fields even exists in this form.  And we definitively
need to do things about the iter going ahead.  In other words we have
to build up a huge abstraction here first.

> > Additionally
> > having fully BTF-enabled toolchains in test VMs is not great either.
> 
> Are you referring to the old BCC toolchain requirements [1]? This is
> solved in CO-RE [2]. The toolchain (Clang/LLVM, pahole) stays on the
> build host; the test VM only needs the prebuilt BPF object, libbpf at
> runtime, and the kernel's own BTF (CONFIG_DEBUG_INFO_BTF). No compiler
> or BTF toolchain is required inside the VM. Clang/LLVM 10+ is enough to
> build CO-RE libbpf tools [3].

libbpf and BTF support is what I mean.  And of course whatever code
you write to communicate with it as a simple shell script won't work
any more.

> Are you referring to the bio sector range comparison in
> __blk_error_inject()?

Yes.

> I don't think that needs to be delegated to a
> BPF map (Documentation/bpf/maps.rst). The ebpf side has direct access
> to the bio fields, so it can apply the same sector/op filtering
> __blk_error_inject() does today. That match is already a linear list
> walk, so the ebpf program just runs the same [start, end] condition
> check.

Which means I need to write eBPF code for every specific range I
want to inject, which makes the thing very hard to use.

> In summary: what do you think of evolving this series
> into eBPF, but BPF_PROG_TYPE_STRUCT_OPS instead of
> ALLOW_ERROR_INJECTION/bpf_override_return()?.

As said before, I prototyped it and it was a mess.  Having about 300
lines of simple code that can be directly used from a shell script
seems much preferable.


      reply	other threads:[~2026-06-05  7:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-02  5:45 configurable block error injection Christoph Hellwig
2026-06-02  5:45 ` [PATCH 1/9] block: remove ALLOW_ERROR_INJECTION for should_fail_bio Christoph Hellwig
2026-06-02  5:45 ` [PATCH 2/9] block: consolidate the calls to should_fail_bio Christoph Hellwig
2026-06-02  5:45 ` [PATCH 3/9] block: refactor should_fail_bio and should_fail_request Christoph Hellwig
2026-06-02  5:45 ` [PATCH 4/9] block: move the FAIL_MAKE_REQUEST symbol from lib/ to block/ Christoph Hellwig
2026-06-02  5:45 ` [PATCH 5/9] block: add a macro to initialize the status table Christoph Hellwig
2026-06-02  5:45 ` [PATCH 6/9] block: add a "tag" for block status codes Christoph Hellwig
2026-06-02  5:45 ` [PATCH 7/9] block: add a str_to_blk_op helper Christoph Hellwig
2026-06-02  5:45 ` [PATCH 8/9] block: add configurable error injection Christoph Hellwig
2026-06-02  9:42   ` Keith Busch
2026-06-02 14:46     ` Christoph Hellwig
2026-06-02 17:56   ` Randy Dunlap
2026-06-02  5:45 ` [PATCH 9/9] block: move the fail request code Christoph Hellwig
2026-06-02  9:43 ` configurable block error injection Keith Busch
2026-06-02  9:58 ` Daniel Gomez
2026-06-02 15:05   ` Christoph Hellwig
2026-06-04 11:06     ` Daniel Gomez
2026-06-05  7:23       ` Christoph Hellwig [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=20260605072338.GA26929@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=bpf@vger.kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=corbet@lwn.net \
    --cc=da.gomez@kernel.org \
    --cc=gost.dev@samsung.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=shinichiro.kawasaki@wdc.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox