Use buffer setter/getter API instead of 'struct arg' to be future proof

[igaw]

During ALPSS Christoph Hellwig, @keithbusch and I had a discussion on the API. As we already experienced the 'struct args' idea to make it future proof is a bit of a hassle. First, we need to versioning the structs and figure out on execution time which version the struct args have. Second, not all function are using struct args.

Christoph suggested we should look at how the SCSI API (kernel?) is handling this. The main trick is to pass around a fixed sized buffer (as defined by the standard) and have a bunch of setters/getters, operating on the buffer. This avoids all the nasty versioning problems and is really future proof (unless the buffer needs to be made bigger but that's also an update of the spec).

Furthermore, we might be able to generate those setters/getters using https://github.com/OpenMPDK/nvme-lint

[ikegami-t]

Seems the scsi data and function struct definitions below suggested.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/scsi/scsi_transport_fc.h#n326

/*
 * FC Remote Port Attributes
 *
 * This structure exists for each remote FC port that a LLDD notifies
 * the subsystem of.  A remote FC port may or may not be a SCSI Target,
 * also be a SCSI initiator, IP endpoint, etc. As such, the remote
 * port is considered a separate entity, independent of "role" (such
 * as scsi target).
 *
 * --
 *
 * Attributes are based on HBAAPI V2.0 definitions. Only those
 * attributes that are determinable by the local port (aka Host)
 * are contained.
 *
 * Fixed attributes are not expected to change. The driver is
 * expected to set these values after successfully calling
 * fc_remote_port_add(). The transport fully manages all get functions
 * w/o driver interaction.
 *
 * Dynamic attributes are expected to change. The driver participates
 * in all get/set operations via functions provided by the driver.
 *
 * Private attributes are transport-managed values. They are fully
 * managed by the transport w/o driver interaction.
 */

struct fc_rport {	/* aka fc_starget_attrs */
	/* Fixed Attributes */
	u32 maxframe_size;
	u32 supported_classes;

	/* Dynamic Attributes */
	u32 dev_loss_tmo;	/* Remote Port loss timeout in seconds. */
	struct fc_fpin_stats fpin_stats;

	/* Private (Transport-managed) Attributes */
	u64 node_name;
	u64 port_name;
...

	/* exported data */
	void *dd_data;			/* Used for driver-specific storage */

	/* internal data */
	unsigned int channel;
	u32 number;
...
} __attribute__((aligned(sizeof(unsigned long))));
...
/* The functions by which the transport class and the driver communicate */
struct fc_function_template {
	void    (*get_rport_dev_loss_tmo)(struct fc_rport *);
	void	(*set_rport_dev_loss_tmo)(struct fc_rport *, u32);

	void	(*get_starget_node_name)(struct scsi_target *);
	void	(*get_starget_port_name)(struct scsi_target *);
...
	unsigned long	show_host_system_hostname:1;

	unsigned long	disable_target_scan:1;
};

[igaw]

The upcoming nvme spec 2.1 will provide an official JSON file which describes all data structures. From this we will generate the API bits and pieces.

There is an early version/prototype of this work to look at: https://github.com/safl/npapi/tree/main/output/npapi

[ikegami-t]

Thanks for sharing the infomation. Noted it. For detail will do check it later.