NACK: [SRU][N/O/P][Pull] Integrate FDLS and fixes patches into Ubuntu

Mon Apr 14 22:44:21 UTC 2025

Hi Micheal,

I outlined some concerns patch-by-patch in-line under the [Fix] section 
of the quoted message below. Most notably, "scsi: fnic: Add and 
integrate support for FIP" fails to apply on Noble.

On 3/25/25 8:40 PM, Michael Reed wrote:
> From: Michael Reed <Michael.Reed at canonical.com>
> 
> SRU Justification
> 
> BugLink: https://bugs.launchpad.net/bugs/2097423
> 
> [Impact]
> The upstream community has recently accepted a big set of changes submitted to the fnic driver. This is a request to pull and integrate these changes into Ubuntu OS.
> 
> This functionality is needed to support port channel RSCN (PC-RSCN)
> handling and serves as a base to create FC-NVME initiators
> (planned later), and eCPU handling (planned later).
> 
> Full details are provided in the following comment:
> https://bugs.launchpad.net/ubuntu/+bug/2097423/comments/17
> 
> [Fix]
> 
> 8697934682f1 scsi: fnic: Propagate SCSI error code from fnic_scsi_drv_init()
> 54428671aac8 scsi: fnic: Test for memory allocation failure and return error code
> 3986001ca11e scsi: fnic: Return appropriate error code from failure of scsi drv init
> 7dbe3aa2f3f8 scsi: fnic: Return appropriate error code for mem alloc failure
> 0620efe789a7 scsi: fnic: Remove always-true IS_FNIC_FCP_INITIATOR macro
> 8ccc5947f5d1 scsi: fnic: Fix use of uninitialized value in debug message
> 17789f8a5b81 scsi: fnic: Delete incorrect debugfs error handling
> bab8551e33f7 scsi: fnic: Remove unnecessary else to fix warning in FDLS FIP
> 6cfba11510d6 scsi: fnic: Remove extern definition from .c files
> 5b6179d4b661 scsi: fnic: Remove unnecessary else and unnecessary break in FDLS
> 8f22f904b25b scsi: fnic: Increment driver version
> 8d26bfcf1d2e scsi: fnic: Add support to handle port channel RSCN
> 7e6886b705fd scsi: fnic: Code cleanup

The backport note here of "resolved patch conflict" is somewhat vague. A 
note that's more specific about what you needed to change would be 
better, or clarification that the conflicts were simple context 
adjustments and not related to changes made by the patch.

> a8650a5eaaf1 scsi: fnic: Add stats and related functionality
> 9243626c211e scsi: fnic: Modify fnic interfaces to use FDLS

Same comment as above regarding the backport note.

> 6335be1c5009 scsi: fnic: Modify IO path to use FDLS
> 9cf9fe2f3ec5 scsi: fnic: Add functionality in fnic to support FDLS

Same comment as above regarding the backport note.

A nitpick: "Backport" in "(Backported from ..." should not be capitalized.

> 098585aa8aca scsi: fnic: Add and integrate support for FIP

Same comment as above regarding the backport note.

Additionally, on Noble, the above patch fails to apply because of a 
recent upstream stable patch "scsi: fnic: Move flush_work initialization 
out of if block".

A nitpick: "Backport" in "(Backported from ..." should not be capitalized.

Jacob

> 09c1e6ab4ab2 scsi: fnic: Add and integrate support for FDMI
> 2c77081969ee scsi: fnic: Add Cisco hardware model names
> f828af44b8dd scsi: fnic: Add support for unsolicited requests and responses
> b5a57f153bdf scsi: fnic: Add support for target based solicited requests and responses
> a63e78eb2b0f scsi: fnic: Add support for fabric based solicited requests and responses
> d859d05ceb3d scsi: fnic: Add headers and definitions for FDLS
> e2813fc27d27 scsi: fnic: Replace shost_printk() with dev_info()/dev_err()
> 
> [Test Plan]
> 
> Sanity Tests
> 
> IOs + link flaps
> IOs + zone in zone out of targets
> 
> [Where problems could occur]
> 
> Local boot: Fibre channel links don't come up.
> 
> SAN boot: FC links don't come up, and the system fails to boot.
> 
> There is some dependency on the storage midlayer. But all changes specified here are _only_ w.r.t the fnic driver. There are no changes anywhere else. The regression risk to the kernel is low.
> 
> [Other Info]
> 
> Noble
> https://code.launchpad.net/~mreed8855/ubuntu/+source/linux/+git/noble/+ref/fnic_lp_2097423
> 
> Oracular
> https://code.launchpad.net/~mreed8855/ubuntu/+source/linux/+git/oracular/+ref/fnic_lp_2097423
> 
> The following changes since commit f6d288e2e91f70dfd1ca845da90fb3ba76f010ea:
> 
>    scsi: fnic: Replace shost_printk() with dev_info()/dev_err() (2025-02-07 10:40:17 -0600)
> 
> are available in the Git repository at:
> 
>    https://code.launchpad.net/~mreed8855/ubuntu/+source/linux/+git/noble/+ref/fnic_lp_2097423
> 
> for you to fetch changes up to 666906269ea3e5fc339515f4b14249e8957fa199:
> 
>    scsi: fnic: Propagate SCSI error code from fnic_scsi_drv_init() (2025-02-07 11:49:28 -0600)
> 
> ----------------------------------------------------------------
> Arun Easi (2):
>        scsi: fnic: Remove always-true IS_FNIC_FCP_INITIATOR macro
>        scsi: fnic: Propagate SCSI error code from fnic_scsi_drv_init()
> 
> Dan Carpenter (1):
>        scsi: fnic: Delete incorrect debugfs error handling
> 
> Dheeraj Reddy Jonnalagadda (1):
>        scsi: fnic: Fix use of uninitialized value in debug message
> 
> Karan Tilak Kumar (20):
>        scsi: fnic: Add headers and definitions for FDLS
>        scsi: fnic: Add support for fabric based solicited requests and responses
>        scsi: fnic: Add support for target based solicited requests and responses
>        scsi: fnic: Add support for unsolicited requests and responses
>        scsi: fnic: Add Cisco hardware model names
>        scsi: fnic: Add and integrate support for FDMI
>        scsi: fnic: Add and integrate support for FIP
>        scsi: fnic: Add functionality in fnic to support FDLS
>        scsi: fnic: Modify IO path to use FDLS
>        scsi: fnic: Modify fnic interfaces to use FDLS
>        scsi: fnic: Add stats and related functionality
>        scsi: fnic: Code cleanup
>        scsi: fnic: Add support to handle port channel RSCN
>        scsi: fnic: Increment driver version
>        scsi: fnic: Remove unnecessary else and unnecessary break in FDLS
>        scsi: fnic: Remove extern definition from .c files
>        scsi: fnic: Remove unnecessary else to fix warning in FDLS FIP
>        scsi: fnic: Return appropriate error code for mem alloc failure
>        scsi: fnic: Return appropriate error code from failure of scsi drv init
>        scsi: fnic: Test for memory allocation failure and return error code
> 
>   drivers/scsi/fnic/Makefile                |    5 +-
>   drivers/scsi/fnic/fdls_disc.c             | 4997 +++++++++++++++++++++++++++++
>   drivers/scsi/fnic/fdls_fc.h               |  253 ++
>   drivers/scsi/fnic/fip.c                   | 1005 ++++++
>   drivers/scsi/fnic/fip.h                   |  159 +
>   drivers/scsi/fnic/fnic.h                  |  288 +-
>   drivers/scsi/fnic/fnic_attrs.c            |   13 +-
>   drivers/scsi/fnic/fnic_debugfs.c          |   11 +-
>   drivers/scsi/fnic/fnic_fcs.c              | 1742 +++++-----
>   drivers/scsi/fnic/fnic_fdls.h             |  434 +++
>   drivers/scsi/fnic/fnic_fip.h              |   48 -
>   drivers/scsi/fnic/fnic_io.h               |   14 +-
>   drivers/scsi/fnic/fnic_isr.c              |   28 +-
>   drivers/scsi/fnic/fnic_main.c             |  675 ++--
>   drivers/scsi/fnic/fnic_pci_subsys_devid.c |  131 +
>   drivers/scsi/fnic/fnic_res.c              |   30 +-
>   drivers/scsi/fnic/fnic_scsi.c             | 1163 ++++---
>   drivers/scsi/fnic/fnic_stats.h            |   49 +-
>   drivers/scsi/fnic/fnic_trace.c            |   81 +-
>   19 files changed, 9278 insertions(+), 1848 deletions(-)
>   create mode 100644 drivers/scsi/fnic/fdls_disc.c
>   create mode 100644 drivers/scsi/fnic/fdls_fc.h
>   create mode 100644 drivers/scsi/fnic/fip.c
>   create mode 100644 drivers/scsi/fnic/fip.h
>   create mode 100644 drivers/scsi/fnic/fnic_fdls.h
>   delete mode 100644 drivers/scsi/fnic/fnic_fip.h
>   create mode 100644 drivers/scsi/fnic/fnic_pci_subsys_devid.c
> 