Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[Christoph Hellwig]

On Tue, Jan 16, 2024 at 03:27:27PM -0800, Alan Adamson wrote:
> It has been requested that the NVMe fault injector be able to inject faults when accessing
> specific Logical Block Addresses (LBA).

Curious, but who has requested this?  Because injecting errors really
isn't the drivers job.

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[alan.adamson]

On 1/17/24 11:24 PM, Christoph Hellwig wrote:
> On Tue, Jan 16, 2024 at 03:27:27PM -0800, Alan Adamson wrote:
>> It has been requested that the NVMe fault injector be able to inject faults when accessing
>> specific Logical Block Addresses (LBA).
> Curious, but who has requested this?  Because injecting errors really
> isn't the drivers job.


It's an application (database) that is requesting it for their error 
handling testing.

Alan

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[Keith Busch]

On Thu, Jan 18, 2024 at 09:02:43AM -0800, alan.adamson@oracle.com wrote:
> 
> On 1/17/24 11:24 PM, Christoph Hellwig wrote:
> > On Tue, Jan 16, 2024 at 03:27:27PM -0800, Alan Adamson wrote:
> > > It has been requested that the NVMe fault injector be able to inject faults when accessing
> > > specific Logical Block Addresses (LBA).
> > Curious, but who has requested this?  Because injecting errors really
> > isn't the drivers job.
> 
> 
> It's an application (database) that is requesting it for their error
> handling testing.

Going out on a limb here... This seems so obscure to burden a driver to
synthesize. Could we just give a hook for a bpf override and you can
totally go for whatever scenario you can imagine with a script?

---
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 50818dbcfa1ae..df87a63335aa8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -381,9 +381,22 @@ static inline void nvme_end_req_zoned(struct request *req)
 	}
 }
 
+#ifdef CONFIG_BPF_KPROBE_OVERRIDE
+static noinline blk_status_t nvme_req_status(struct nvme_request *req)
+{
+	return nvme_error_status(req->status);
+}
+ALLOW_ERROR_INJECTION(nvme_req_status, ERRNO);
+#else
+static inline blk_status_t nvme_req_status(struct nvme_request *req)
+{
+	return nvme_error_status(req->status);
+}
+#endif
+
 static inline void nvme_end_req(struct request *req)
 {
-	blk_status_t status = nvme_error_status(nvme_req(req)->status);
+	blk_status_t status = nvme_req_status(nvme_req(req));
 
 	if (unlikely(nvme_req(req)->status && !(req->rq_flags & RQF_QUIET)))
 		nvme_log_error(req);
--

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[Christoph Hellwig]

On Thu, Jan 18, 2024 at 09:02:43AM -0800, alan.adamson@oracle.com wrote:
>
> On 1/17/24 11:24 PM, Christoph Hellwig wrote:
>> On Tue, Jan 16, 2024 at 03:27:27PM -0800, Alan Adamson wrote:
>>> It has been requested that the NVMe fault injector be able to inject faults when accessing
>>> specific Logical Block Addresses (LBA).
>> Curious, but who has requested this?  Because injecting errors really
>> isn't the drivers job.
>
>
> It's an application (database) that is requesting it for their error 
> handling testing.

Well, how about they then insert it into the real or virtual hardware.
The Linux nvme driver isn't really an error injection framework.

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[alan.adamson]

On 1/23/24 1:05 AM, Christoph Hellwig wrote:
> On Thu, Jan 18, 2024 at 09:02:43AM -0800, alan.adamson@oracle.com wrote:
>> On 1/17/24 11:24 PM, Christoph Hellwig wrote:
>>> On Tue, Jan 16, 2024 at 03:27:27PM -0800, Alan Adamson wrote:
>>>> It has been requested that the NVMe fault injector be able to inject faults when accessing
>>>> specific Logical Block Addresses (LBA).
>>> Curious, but who has requested this?  Because injecting errors really
>>> isn't the drivers job.
>>
>> It's an application (database) that is requesting it for their error
>> handling testing.
> Well, how about they then insert it into the real or virtual hardware.
> The Linux nvme driver isn't really an error injection framework.
>
Sorry if you receive multiple of these, I'm having email/sending issues.


I get it, but there is already an injection framework in place for nvme. 
Is there no plan to improve it?

Alan

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[Christoph Hellwig]

On Tue, Jan 23, 2024 at 09:25:51AM -0800, alan.adamson@oracle.com wrote:
> I get it, but there is already an injection framework in place for nvme. Is 
> there no plan to improve it?

Injecting fake I/O error really isn't the driver job.  For block
I/O we could do it the block layer or DM, but that's not something
to add to random drivers.  Error injection makes sense for testing
the driver itself, not applications.

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[alan.adamson]

On 1/24/24 1:04 AM, Christoph Hellwig wrote:
> On Tue, Jan 23, 2024 at 09:25:51AM -0800, alan.adamson@oracle.com wrote:
>> I get it, but there is already an injection framework in place for nvme. Is
>> there no plan to improve it?
> Injecting fake I/O error really isn't the driver job.  For block
> I/O we could do it the block layer or DM, but that's not something
> to add to random drivers.  Error injection makes sense for testing
> the driver itself, not applications.

Thanks, I'll look at the block layer for this.  Do you think adding 
error injection to qemu-nvme makes sense?

Alan

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[Keith Busch]

On Wed, Jan 24, 2024 at 08:52:54AM -0800, alan.adamson@oracle.com wrote:
> 
> Thanks, I'll look at the block layer for this.  Do you think adding error
> injection to qemu-nvme makes sense?

I frequently added custom error injection to qemu, though it was always
pretty hacky so I never upstreamed anything. Klaus may be interested in
getting a feature like that integrated upstream if you're able to put
something together.

Re: [RFC 0/1] nvme: Add NVMe LBA Fault Injection

[alan.adamson]

On 1/24/24 8:59 AM, Keith Busch wrote:
> On Wed, Jan 24, 2024 at 08:52:54AM -0800, alan.adamson@oracle.com wrote:
>> Thanks, I'll look at the block layer for this.  Do you think adding error
>> injection to qemu-nvme makes sense?
> I frequently added custom error injection to qemu, though it was always
> pretty hacky so I never upstreamed anything. Klaus may be interested in
> getting a feature like that integrated upstream if you're able to put
> something together.

Thanks, I'll put something together.

Alan