Hello Muneendra,
On Thu, 2023-09-14 at 14:55 -0700, Muneendra Kumar wrote:
> From: Muneendra <muneendra.kumar@xxxxxxxxxxxx>
>
> This patch adds the support to handle FPIN-Li for FC-NVMe.
> On receiving the FPIN-Li events this patch moves the devices paths
> which are affected due to link integrity to marginal path groups.
> The paths which are set to marginal path group will be unset
> on receiving the RSCN events
>
> Signed-off-by: Muneendra <muneendra.kumar@xxxxxxxxxxxx>
Thanks! This looks mostly good to me, some comments below.
> ---
> multipathd/fpin_handlers.c | 177 ++++++++++++++++++++++++++++-------
> --
> 1 file changed, 136 insertions(+), 41 deletions(-)
>
> diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c
> index aa0f63c9..94f390ec 100644
> --- a/multipathd/fpin_handlers.c
> +++ b/multipathd/fpin_handlers.c
> @@ -203,61 +203,153 @@ fpin_add_marginal_dev_info(uint32_t host_num,
> char *devname)
> }
>
> /*
> - * This function goes through the vecs->pathvec, and for
> - * each path, check that the host number,
> - * the target WWPN associated with the path matches
> - * with the els wwpn and sets the path and port state to
> + * This function compares Transport Address Controller Port pn,
> + * Host Transport Address Controller Port pn with the els wwpn
> ,attached_wwpn
> + * and return 0 on success
> + */
> +static int extract_nvme_addresses_chk_path_pwwn(const char
> *address,
> + uint64_t els_wwpn, uint64_t els_attached_wwpn)
> +
> +{
> + uint64_t traddr;
> + uint64_t host_traddr;
> +
> + /*
> + * Find the position of "traddr=" and "host_traddr="
> + * and the address will be in the below format
> + * "traddr=nn-0x200400110dff9400:pn-0x200400110dff9400,
> + * host_traddr=nn-0x200400110dff9400:pn-0x200400110dff9400"
> + */
> + const char *traddr_start = strstr(address, "traddr=");
> + const char *host_traddr_start = strstr(address,
> "host_traddr=");
> +
> + if (!traddr_start || !host_traddr_start)
> + return -EINVAL;
> +
> + /* Extract traddr pn */
> + if (sscanf(traddr_start, "traddr=nn-%*[^:]:pn-%" SCNx64,
> &traddr) != 1)
> + return -EINVAL;
> +
> + /* Extract host_traddr pn*/
> + if (sscanf(host_traddr_start, "host_traddr=nn-%*[^:]:pn-%"
> SCNx64,
> + &host_traddr) != 1)
> + return -EINVAL;
> + condlog(4, "traddr 0x%lx hosttraddr 0x%lx els_wwpn 0x%lx
> els_host_traddr 0x%lx",
> + traddr, host_traddr,
> + els_wwpn, els_attached_wwpn);
> + if ((host_traddr == els_attached_wwpn) && (traddr ==
> els_wwpn))
> + return 0;
> + return -EINVAL;
Please don't return -EINVAL to indicate that the addresses don't match.
The function should return 1 (match) or 0 (no match) or a negative
error code if something is actually wrong.
> +}
> +
> +/*
> + * This function check that the Transport Address Controller Port
> pn,
> + * Host Transport Address Controller Port pn associated with the
> path matches
> + * with the els wwpn ,attached_wwpn and sets the path state to
> * Marginal
> */
> -static int fpin_chk_wwn_setpath_marginal(uint16_t host_num, struct
> vectors *vecs,
> +static void fpin_check_set_nvme_path_marginal(uint16_t host_num,
> struct path *pp,
> + uint64_t els_wwpn, uint64_t attached_wwpn)
> +{
> + struct udev_device *ctl = NULL;
> + const char *address = NULL;
> + int ret = 0;
> +
> + ctl = udev_device_get_parent_with_subsystem_devtype(pp->udev,
> "nvme", NULL);
> + if (ctl == NULL) {
> + condlog(2, "%s: No parent device for ", pp->dev);
> + return;
> + }
> + address = udev_device_get_sysattr_value(ctl, "address");
> + if (!address) {
> + condlog(2, "%s: unable to get the address ", pp-
> >dev);
> + return;
> + }
> + condlog(4, "\n address %s: dev :%s\n", address, pp->dev);
> + ret = extract_nvme_addresses_chk_path_pwwn(address, els_wwpn,
> attached_wwpn);
> + if (ret < 0)
> + return;
> + ret = fpin_path_setmarginal(pp);
> + if (ret < 0)
> + return;
> + fpin_add_marginal_dev_info(host_num, pp->dev);
I think that you should call fpin_add_marginal_dev_info() first, and
only set the path to marginal state if it has succeeded.
I realize the same applies to the SCSI code flow.
Regards,
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
