Re: [PATCH v2 7/7] nvme: fcloop: Add set_marginal_rport to sysfs

[Hannes Reinecke <hare@suse.de>]

On 9/2/26 10:05 PM, Jesse Taube wrote:
> To allow testing of multipath failover, add a sysfs attribute to set a
> remote port as marginal. This will allow the fcloop LLDD to set the
> marginal flag on a remote port, simulating a marginal link.
> 
> Example:
> Turn on marginal for a remote port matching wwnn and wwpn:
> `echo 'wwnn=0x200000109b5f2956,wwpn=0x100000109b5f2956,marginal=1' >
> /sys/class/fcloop/ctl/set_marginal_rport`
> 
> Turn off marginal for a remote port matching wwnn and wwpn:
> `echo 'wwnn=0x200000109b5f2956,wwpn=0x100000109b5f2956,marginal=0' >
> /sys/class/fcloop/ctl/set_marginal_rport`
> 
> Suggested-by: John Meneghini <jmeneghi@redhat.com>
> Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
> V10 -> V1:
>   - New patch
> V1 -> V2:
>   - Fix reference count leak in error path
> ---
>   drivers/nvme/target/fcloop.c | 50 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index b63af3b643a6..9977c2a71bd5 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -6,6 +6,7 @@
>   #include <linux/module.h>
>   #include <linux/parser.h>
>   #include <uapi/scsi/fc/fc_fs.h>
> +#include <uapi/scsi/fc/fc_els.h>
>   
>   #include "../host/nvme.h"
>   #include "../target/nvmet.h"
> @@ -21,6 +22,7 @@ enum {
>   	NVMF_OPT_FCADDR		= 1 << 3,
>   	NVMF_OPT_LPWWNN		= 1 << 4,
>   	NVMF_OPT_LPWWPN		= 1 << 5,
> +	NVMF_OPT_MARGINAL	= 1 << 6,
>   };
>   
>   struct fcloop_ctrl_options {
> @@ -31,6 +33,7 @@ struct fcloop_ctrl_options {
>   	u32			fcaddr;
>   	u64			lpwwnn;
>   	u64			lpwwpn;
> +	u32			marginal;
>   };
>   
>   static const match_table_t opt_tokens = {
> @@ -40,6 +43,7 @@ static const match_table_t opt_tokens = {
>   	{ NVMF_OPT_FCADDR,	"fcaddr=%x"	},
>   	{ NVMF_OPT_LPWWNN,	"lpwwnn=%s"	},
>   	{ NVMF_OPT_LPWWPN,	"lpwwpn=%s"	},
> +	{ NVMF_OPT_MARGINAL,	"marginal=%d"	},
>   	{ NVMF_OPT_ERR,		NULL		}
>   };
>   
> @@ -120,6 +124,13 @@ fcloop_parse_options(struct fcloop_ctrl_options *opts,
>   			}
>   			opts->lpwwpn = token64;
>   			break;
> +		case NVMF_OPT_MARGINAL:
> +			if (match_int(args, &token)) {
> +				ret = -EINVAL;
> +				goto out_free_options;
> +			}
> +			opts->marginal = token;
> +			break;
>   		default:
>   			pr_warn("unknown parameter or missing value '%s'\n", p);
>   			ret = -EINVAL;
> @@ -199,6 +210,9 @@ fcloop_parse_nm_options(struct device *dev, u64 *nname, u64 *pname,
>   
>   #define TGTPORT_OPTS	(NVMF_OPT_WWNN | NVMF_OPT_WWPN)
>   
> +#define MARGINAL_OPTS	(NVMF_OPT_WWNN | NVMF_OPT_WWPN | \
> +			 NVMF_OPT_MARGINAL)
> +
>   
>   static DEFINE_SPINLOCK(fcloop_lock);
>   static LIST_HEAD(fcloop_lports);
> @@ -1663,6 +1677,40 @@ fcloop_set_cmd_drop(struct device *dev, struct device_attribute *attr,
>   	return count;
>   }
>   
> +static ssize_t
> +fcloop_set_marginal_rport(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct fcloop_nport *nport;
> +	struct fcloop_ctrl_options opts = {};
> +	unsigned long flags;
> +	int ret;
> +
> +	ret = fcloop_parse_options(&opts, buf);
> +	if (ret)
> +		return ret;
> +
> +	/* everything there ? */
> +	if ((opts.mask & MARGINAL_OPTS) != MARGINAL_OPTS)
> +		return -EINVAL;
> +
> +	nport = fcloop_nport_lookup(opts.wwnn, opts.wwpn);
> +	if (!nport)
> +		return -ENOENT;
> +
> +	spin_lock_irqsave(&fcloop_lock, flags);
> +	if (!nport->rport || !nport->rport->remoteport) {
> +		spin_unlock_irqrestore(&fcloop_lock, flags);
> +		fcloop_nport_put(nport);
> +		return -ENOENT;
> +	}
> +
> +	nvme_fc_set_remoteport_fpin(nport->rport->remoteport, opts.marginal);
> +	spin_unlock_irqrestore(&fcloop_lock, flags);
> +	fcloop_nport_put(nport);
> +
> +	return count;
> +}
>   
>   static DEVICE_ATTR(add_local_port, 0200, NULL, fcloop_create_local_port);
>   static DEVICE_ATTR(del_local_port, 0200, NULL, fcloop_delete_local_port);
> @@ -1671,6 +1719,7 @@ static DEVICE_ATTR(del_remote_port, 0200, NULL, fcloop_delete_remote_port);
>   static DEVICE_ATTR(add_target_port, 0200, NULL, fcloop_create_target_port);
>   static DEVICE_ATTR(del_target_port, 0200, NULL, fcloop_delete_target_port);
>   static DEVICE_ATTR(set_cmd_drop, 0200, NULL, fcloop_set_cmd_drop);
> +static DEVICE_ATTR(set_marginal_rport, 0200, NULL, fcloop_set_marginal_rport);
>   
>   static struct attribute *fcloop_dev_attrs[] = {
>   	&dev_attr_add_local_port.attr,
> @@ -1680,6 +1729,7 @@ static struct attribute *fcloop_dev_attrs[] = {
>   	&dev_attr_add_target_port.attr,
>   	&dev_attr_del_target_port.attr,
>   	&dev_attr_set_cmd_drop.attr,
> +	&dev_attr_set_marginal_rport.attr,
>   	NULL
>   };
>   
Please don't make it a fabrics option. Setting is via sysfs is 
sufficient seeing it's for testing only.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich