| Message ID | 20210331201154.20348-1-emilne@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi_dh_alua: remove check for ASC 24h when ILLEGAL_REQUEST returned on RTPG w/extended header | expand |
On 2021-03-31 4:11 p.m., Ewan D. Milne wrote: > Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't support the > extended header, so remove the check for INVALID FIELD IN CDB. Wow. Referring to the 18 byte sense buffer as an extended header sounds like it comes from the transition of SCSI-1 to SCSI-2, about 30 years ago. Those arrays need to be named and shamed. Doug Gilbert Hmm, it is April first ... > Signed-off-by: Ewan D. Milne <emilne@redhat.com> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index e42390333c6e..c4c2f23cf79f 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -587,10 +587,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) > * even though it shouldn't according to T10. > * The retry without rtpg_ext_hdr_req set > * handles this. > + * Note: some arrays return a sense key of ILLEGAL_REQUEST > + * with ASC 00h if they don't support the extended header. > */ > if (!(pg->flags & ALUA_RTPG_EXT_HDR_UNSUPP) && > - sense_hdr.sense_key == ILLEGAL_REQUEST && > - sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) { > + sense_hdr.sense_key == ILLEGAL_REQUEST) { > pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; > goto retry; > } >
On 3/31/21 10:11 PM, Ewan D. Milne wrote: > Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't support the > extended header, so remove the check for INVALID FIELD IN CDB. > > Signed-off-by: Ewan D. Milne <emilne@redhat.com> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index e42390333c6e..c4c2f23cf79f 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -587,10 +587,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) > * even though it shouldn't according to T10. > * The retry without rtpg_ext_hdr_req set > * handles this. > + * Note: some arrays return a sense key of ILLEGAL_REQUEST > + * with ASC 00h if they don't support the extended header. > */ > if (!(pg->flags & ALUA_RTPG_EXT_HDR_UNSUPP) && > - sense_hdr.sense_key == ILLEGAL_REQUEST && > - sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) { > + sense_hdr.sense_key == ILLEGAL_REQUEST) { > pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; > goto retry; > } > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
On 4/1/21 6:24 AM, Douglas Gilbert wrote: > On 2021-03-31 4:11 p.m., Ewan D. Milne wrote: >> Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't support the >> extended header, so remove the check for INVALID FIELD IN CDB. > > Wow. Referring to the 18 byte sense buffer as an extended header sounds > like it comes from the transition of SCSI-1 to SCSI-2, about 30 years ago. > Those arrays need to be named and shamed. > > Doug Gilbert > Hmm, it is April first ... > Hey, it took us some time to even _get_ this header; originally the specification didn't have that, requiring us to second-guess how long the array might take for a transition. But still, April the first :-) Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
On Thu, 2021-04-01 at 00:24 -0400, Douglas Gilbert wrote: > On 2021-03-31 4:11 p.m., Ewan D. Milne wrote: > > Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't > > support the > > extended header, so remove the check for INVALID FIELD IN CDB. > > Wow. Referring to the 18 byte sense buffer as an extended header > sounds > like it comes from the transition of SCSI-1 to SCSI-2, about 30 years > ago. > Those arrays need to be named and shamed. Sorry, I was referring to the RTPG command, not the sense header. cdb[0] = MAINTENANCE_IN; if (!(flags & ALUA_RTPG_EXT_HDR_UNSUPP)) cdb[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT; else cdb[1] = MI_REPORT_TARGET_PGS; The array returned an error when the MI_EXT_HDR_PARAM_FMT bit was set but worked fine without it. It was an older array, it worked with older kernels until we removed the ability to detach the device handler. So RTPG needs to work. -Ewan > > Doug Gilbert > Hmm, it is April first ... > > > Signed-off-by: Ewan D. Milne <emilne@redhat.com> > > --- > > drivers/scsi/device_handler/scsi_dh_alua.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c > > b/drivers/scsi/device_handler/scsi_dh_alua.c > > index e42390333c6e..c4c2f23cf79f 100644 > > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > > @@ -587,10 +587,11 @@ static int alua_rtpg(struct scsi_device > > *sdev, struct alua_port_group *pg) > > * even though it shouldn't according to T10. > > * The retry without rtpg_ext_hdr_req set > > * handles this. > > + * Note: some arrays return a sense key of > > ILLEGAL_REQUEST > > + * with ASC 00h if they don't support the extended > > header. > > */ > > if (!(pg->flags & ALUA_RTPG_EXT_HDR_UNSUPP) && > > - sense_hdr.sense_key == ILLEGAL_REQUEST && > > - sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) { > > + sense_hdr.sense_key == ILLEGAL_REQUEST) { > > pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; > > goto retry; > > } > > > >
On Wed, 31 Mar 2021 16:11:54 -0400, Ewan D. Milne wrote: > Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't support the > extended header, so remove the check for INVALID FIELD IN CDB. Applied to 5.13/scsi-queue, thanks! [1/1] scsi_dh_alua: remove check for ASC 24h when ILLEGAL_REQUEST returned on RTPG w/extended header https://git.kernel.org/mkp/scsi/c/bc3f2b42b70e -- Martin K. Petersen Oracle Linux Engineering
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index e42390333c6e..c4c2f23cf79f 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -587,10 +587,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) * even though it shouldn't according to T10. * The retry without rtpg_ext_hdr_req set * handles this. + * Note: some arrays return a sense key of ILLEGAL_REQUEST + * with ASC 00h if they don't support the extended header. */ if (!(pg->flags & ALUA_RTPG_EXT_HDR_UNSUPP) && - sense_hdr.sense_key == ILLEGAL_REQUEST && - sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) { + sense_hdr.sense_key == ILLEGAL_REQUEST) { pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; goto retry; }
Some arrays return ILLEGAL_REQUEST with ASC 00h if they don't support the extended header, so remove the check for INVALID FIELD IN CDB. Signed-off-by: Ewan D. Milne <emilne@redhat.com> --- drivers/scsi/device_handler/scsi_dh_alua.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)