On 6/8/26 11:30 AM, Dave Marquardt via B4 Relay wrote: > From: Dave Marquardt <[email protected]> > > Add support for basic FPIN messages to the ibmvfc driver. This includes > > - adding FPIN handling support to the async event handler > - offloading processing of FPIN messages to a work queue > - converting the VIOS FPIN message to a struct fc_els_fpin as used by > the Linux kernel > - passing the converted struct fc_els_fpin to fc_host_fpin_rcv for > processing > > The FPIN message conversion routines include a common routine that > will also be used in patches 6 and 7, which add full and extended FPIN > support. > --- > drivers/scsi/Kconfig | 10 ++ > drivers/scsi/ibmvscsi/Makefile | 1 + > drivers/scsi/ibmvscsi/ibmvfc.c | 226 > ++++++++++++++++++++++++++++++++++- > drivers/scsi/ibmvscsi/ibmvfc.h | 15 +++ > drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 131 ++++++++++++++++++++ > 5 files changed, 379 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index c3042393af23..d5fc7eb2ebb1 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -758,6 +758,16 @@ config SCSI_IBMVFC > To compile this driver as a module, choose M here: the > module will be called ibmvfc. > > +config SCSI_IBMVFC_KUNIT_TEST > + tristate "KUnit tests for the IBM POWER Virtual FC Client" if > !KUNIT_ALL_TESTS > + depends on SCSI_IBMVFC && KUNIT > + default KUNIT_ALL_TESTS > + help > + Compile IBM POWER Virtual FC client KUnit tests. These tests > + specifically test FPIN functionality. To compile this driver > + as a module, choose M here: the module will be called > + ibmvfc_kunit. > + > config SCSI_IBMVFC_TRACE > bool "enable driver internal trace" > depends on SCSI_IBMVFC > diff --git a/drivers/scsi/ibmvscsi/Makefile b/drivers/scsi/ibmvscsi/Makefile > index 5eb1cb1a0028..75dc7aee15a0 100644 > --- a/drivers/scsi/ibmvscsi/Makefile > +++ b/drivers/scsi/ibmvscsi/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi.o > obj-$(CONFIG_SCSI_IBMVFC) += ibmvfc.o > +obj-$(CONFIG_SCSI_IBMVFC_KUNIT_TEST) += ibmvfc_kunit.o > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c > index 3dd2adda195e..9e5f0c0f0369 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c > @@ -30,6 +30,9 @@ > #include <scsi/scsi_tcq.h> > #include <scsi/scsi_transport_fc.h> > #include <scsi/scsi_bsg_fc.h> > +#include <kunit/visibility.h> > +#include <scsi/fc/fc_els.h> > +#include <linux/overflow.h> > #include "ibmvfc.h" > > static unsigned int init_timeout = IBMVFC_INIT_TIMEOUT; > @@ -3137,6 +3140,7 @@ static const struct ibmvfc_async_desc ae_desc [] = { > { "Halt", IBMVFC_AE_HALT, IBMVFC_DEFAULT_LOG_LEVEL }, > { "Resume", IBMVFC_AE_RESUME, IBMVFC_DEFAULT_LOG_LEVEL }, > { "Adapter Failed", IBMVFC_AE_ADAPTER_FAILED, IBMVFC_DEFAULT_LOG_LEVEL > }, > + { "FPIN", IBMVFC_AE_FPIN, IBMVFC_DEFAULT_LOG_LEVEL }, > }; > > static const struct ibmvfc_async_desc unknown_ae = { > @@ -3185,17 +3189,211 @@ static const char *ibmvfc_get_link_state(enum > ibmvfc_ae_link_state state) > return ""; > } > > +#define IBMVFC_FPIN_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + > sizeof(struct fc_fn_congn_desc)) > +#define IBMVFC_FPIN_LI_DESC_SZ (sizeof(struct fc_els_fpin) + \ > + struct_size_t(struct fc_fn_li_desc, pname_list, > 1)) > +#define IBMVFC_FPIN_PEER_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + \ > + struct_size_t(struct > fc_fn_peer_congn_desc, pname_list, 1)) > + > +/** > + * ibmvfc_fpin_size_helper(): compute fpin structure size based on fpin > status > + * @fpin_status: status value > + * > + * Return: > + * 0: invalid fpin_status > + * other: valid size > + */ > +static size_t ibmvfc_fpin_size_helper(u8 fpin_status) > +{ > + size_t size = 0; > + > + switch (fpin_status) { > + case IBMVFC_AE_FPIN_LINK_CONGESTED: > + case IBMVFC_AE_FPIN_CONGESTION_CLEARED: > + size = IBMVFC_FPIN_CONGN_DESC_SZ; > + break; > + case IBMVFC_AE_FPIN_PORT_CONGESTED: > + case IBMVFC_AE_FPIN_PORT_CLEARED: > + size = IBMVFC_FPIN_PEER_CONGN_DESC_SZ; > + break; > + case IBMVFC_AE_FPIN_PORT_DEGRADED: > + size = IBMVFC_FPIN_LI_DESC_SZ; > + break; > + default: > + break; > + } > + > + return size; > +} > + > +/** > + * ibmvfc_common_fpin_to_desc(): allocate and populate a struct fc_els_fpin > struct > + * containing a descriptor. > + * > + * Allocate a struct fc_els_fpin containing a descriptor and populate > + * based on data from *ibmvfc_fpin. > + * > + * Return: > + * NULL - unable to allocate structure > + * non-NULL - pointer to populated struct fc_els_fpin > + */ > +static struct fc_els_fpin * > +ibmvfc_common_fpin_to_desc(u8 fpin_status, __be64 wwpn, __be16 modifier, > + __be32 period, __be32 threshold, __be32 event_count) > +{ > + struct fc_fn_peer_congn_desc *pdesc; > + struct fc_fn_congn_desc *cdesc; > + struct fc_fn_li_desc *ldesc; > + struct fc_els_fpin *fpin; > + size_t size; > + > + size = ibmvfc_fpin_size_helper(fpin_status); > + if (size == 0) > + return NULL; > + > + fpin = kzalloc(size, GFP_KERNEL); > + if (fpin == NULL) > + return NULL; > + > + fpin->fpin_cmd = ELS_FPIN; > + > + switch (fpin_status) { > + case IBMVFC_AE_FPIN_CONGESTION_CLEARED: > + case IBMVFC_AE_FPIN_LINK_CONGESTED: > + fpin->desc_len = cpu_to_be32(sizeof(struct fc_fn_congn_desc)); > + cdesc = (struct fc_fn_congn_desc *)fpin->fpin_desc; > + cdesc->desc_tag = cpu_to_be32(ELS_DTAG_CONGESTION); > + cdesc->desc_len = > cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*cdesc)); > + if (fpin_status == IBMVFC_AE_FPIN_CONGESTION_CLEARED) > + cdesc->event_type = cpu_to_be16(FPIN_CONGN_CLEAR); > + else > + cdesc->event_type = cpu_to_be16(FPIN_CONGN_DEVICE_SPEC); > + cdesc->event_modifier = modifier; > + cdesc->event_period = period; > + cdesc->severity = FPIN_CONGN_SEVERITY_WARNING; > + break; > + case IBMVFC_AE_FPIN_PORT_CONGESTED: > + case IBMVFC_AE_FPIN_PORT_CLEARED: > + fpin->desc_len = > + cpu_to_be32(struct_size_t(struct fc_fn_peer_congn_desc, > pname_list, 1)); > + pdesc = (struct fc_fn_peer_congn_desc *)fpin->fpin_desc; > + pdesc->desc_tag = cpu_to_be32(ELS_DTAG_PEER_CONGEST); > + pdesc->desc_len = > cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*pdesc)); > + if (fpin_status == IBMVFC_AE_FPIN_PORT_CLEARED) > + pdesc->event_type = cpu_to_be16(FPIN_CONGN_CLEAR); > + else > + pdesc->event_type = cpu_to_be16(FPIN_CONGN_DEVICE_SPEC); > + pdesc->event_modifier = modifier; > + pdesc->event_period = period; > + pdesc->detecting_wwpn = cpu_to_be64(0); > + pdesc->attached_wwpn = wwpn; > + pdesc->pname_count = cpu_to_be32(1); > + pdesc->pname_list[0] = wwpn; > + break; > + case IBMVFC_AE_FPIN_PORT_DEGRADED: > + fpin->desc_len = cpu_to_be32(struct_size_t(struct > fc_fn_li_desc, pname_list, 1)); > + ldesc = (struct fc_fn_li_desc *)fpin->fpin_desc; > + ldesc->desc_tag = cpu_to_be32(ELS_DTAG_LNK_INTEGRITY); > + ldesc->desc_len = > cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*ldesc)); > + ldesc->event_type = cpu_to_be16(FPIN_LI_UNKNOWN); > + ldesc->event_modifier = modifier; > + ldesc->event_threshold = threshold; > + ldesc->event_count = event_count; > + ldesc->detecting_wwpn = cpu_to_be64(0); > + ldesc->attached_wwpn = wwpn; > + ldesc->pname_count = cpu_to_be32(1); > + ldesc->pname_list[0] = wwpn; > + break; > + default: > + /* This should be caught above. */ > + kfree(fpin); > + fpin = NULL; > + break; > + } > + > + return fpin; > +} > + > +/** > + * ibmvfc_basic_fpin_to_desc(): allocate and populate a struct fc_els_fpin > struct > + * containing a descriptor. > + * @ibmvfc_fpin: Pointer to async crq > + * > + * Allocate a struct fc_els_fpin containing a descriptor and populate > + * based on data from *ibmvfc_fpin. > + * > + * Return: > + * NULL - unable to allocate structure > + * non-NULL - pointer to populated struct fc_els_fpin > + */ > +static struct fc_els_fpin * > +ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn) > +{ > + return ibmvfc_common_fpin_to_desc(crq->fpin_status, cpu_to_be64(wwpn), > + cpu_to_be16(0), > + > cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_PERIOD), > + > cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD), > + cpu_to_be32(1)); > +} > + > +/** > + * ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work > queue > + * @work: pointer to work_struct > + */ > +static void ibmvfc_process_async_work(struct work_struct *work) > +{ > + struct ibmvfc_async_work *aw; > + struct ibmvfc_async_crq *crq; > + struct ibmvfc_target *tgt; > + struct ibmvfc_host *vhost; > + struct fc_els_fpin *fpin; > + > + aw = container_of(work, struct ibmvfc_async_work, async_work_s); > + crq = aw->crq; > + vhost = aw->vhost; > + > + if (!crq->scsi_id && !crq->wwpn && !crq->node_name) > + goto end; > + list_for_each_entry(tgt, &vhost->targets, queue) { > + if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id) > + continue; > + if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn) > + continue; > + if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != > crq->node_name) > + continue; > + if (!tgt->rport) > + continue; > + fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn); > + if (fpin) { > + fc_host_fpin_rcv(tgt->vhost->host, > + sizeof(*fpin) + > be32_to_cpu(fpin->desc_len), > + (char *)fpin, 0); > + kfree(fpin); > + } else > + dev_err_ratelimited(vhost->dev, > + "FPIN event %u received, unable to > process\n", > + crq->fpin_status); > + } > + > + end: > + crq->valid = 0;
I've been mulling this over since last week and I'm inclined to agree with the sashiko analysis as well that clearing this asynchrously is not ideal. I think it's better to make a copy of the crq entry and pass the copy instead of a pointer to the data in our actual ring buffer. -Tyrel
