mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1] nvmet: Add traffic based keep alive support to configfs
@ 2026-09-22 20:22 Jesse Taube
  2026-09-22 22:12 ` Mohamed Khalfella
  0 siblings, 1 reply; 2+ messages in thread
From: Jesse Taube @ 2026-09-22 20:22 UTC (permalink / raw)
  To: linux-nvme
  Cc: Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Martin K. Petersen, John Meneghini, Chris Leech, linux-kernel,
	Mohamed Khalfella, Jesse Taube

Add the ability to turn on and off tbkas (traffic based keep alive
support) via configfs. This is useful for testing and debugging.

Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
---
 drivers/nvme/target/admin-cmd.c |  4 +++-
 drivers/nvme/target/configfs.c  | 27 +++++++++++++++++++++++++++
 drivers/nvme/target/core.c      |  3 ++-
 drivers/nvme/target/nvmet.h     |  1 +
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 7764a3c0195c..e734e4b68784 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -696,7 +696,9 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 	/* XXX: figure out what to do about RTD3R/RTD3 */
 	id->oaes = cpu_to_le32(NVMET_AEN_CFG_OPTIONAL);
-	ctratt = NVME_CTRL_ATTR_HID_128_BIT | NVME_CTRL_ATTR_TBKAS;
+	ctratt = NVME_CTRL_ATTR_HID_128_BIT;
+	if (subsys->tbkas)
+		ctratt |= NVME_CTRL_ATTR_TBKAS;
 	if (nvmet_is_pci_ctrl(ctrl))
 		ctratt |= NVME_CTRL_ATTR_RHII;
 	id->ctratt = cpu_to_le32(ctratt);
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 6286e38436dd..5766e594e724 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1369,6 +1369,32 @@ static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
 }
 CONFIGFS_ATTR(nvmet_subsys_, attr_serial);
 
+static ssize_t nvmet_subsys_attr_tbkas_show(struct config_item *item,
+					     char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n", to_subsys(item)->tbkas);
+}
+
+static ssize_t nvmet_subsys_attr_tbkas_store(struct config_item *item,
+					      const char *page, size_t count)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	bool tbkas;
+
+	if (subsys->subsys_discovered) {
+		pr_err("Can't set traffic based keep alive support. %d is already assigned\n",
+		       subsys->tbkas);
+		return -EINVAL;
+	}
+
+	if (kstrtobool(page, &tbkas))
+		return -EINVAL;
+
+	subsys->tbkas = tbkas;
+	return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, attr_tbkas);
+
 static ssize_t nvmet_subsys_attr_cntlid_min_show(struct config_item *item,
 						 char *page)
 {
@@ -1706,6 +1732,7 @@ static struct configfs_attribute *nvmet_subsys_attrs[] = {
 	&nvmet_subsys_attr_attr_qid_max,
 	&nvmet_subsys_attr_attr_ieee_oui,
 	&nvmet_subsys_attr_attr_firmware,
+	&nvmet_subsys_attr_attr_tbkas,
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 	&nvmet_subsys_attr_attr_pi_enable,
 #endif
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 43871a8f56ca..20aeb9624be0 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1227,7 +1227,7 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_sq *sq,
 		goto fail;
 	}
 
-	if (sq->ctrl)
+	if (sq->ctrl && sq->ctrl->subsys->tbkas)
 		sq->ctrl->reset_tbkas = true;
 
 	return true;
@@ -1865,6 +1865,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 	}
 
 	subsys->ieee_oui = 0;
+	subsys->tbkas = true;
 
 	subsys->firmware_rev = kstrndup(UTS_RELEASE, NVMET_FR_MAX_SIZE, GFP_KERNEL);
 	if (!subsys->firmware_rev) {
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index dbda55895f4f..241eac4fa969 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -343,6 +343,7 @@ struct nvmet_subsys {
 	bool			subsys_discovered;
 	char			*subsysnqn;
 	bool			pi_support;
+	bool			tbkas;
 
 	struct config_group	group;
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v1] nvmet: Add traffic based keep alive support to configfs
  2026-09-22 20:22 [PATCH v1] nvmet: Add traffic based keep alive support to configfs Jesse Taube
@ 2026-09-22 22:12 ` Mohamed Khalfella
  0 siblings, 0 replies; 2+ messages in thread
From: Mohamed Khalfella @ 2026-09-22 22:12 UTC (permalink / raw)
  To: Jesse Taube
  Cc: linux-nvme, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Martin K. Petersen, John Meneghini, Chris Leech, linux-kernel

On Tue 2026-09-22 16:22:56 -0400, Jesse Taube wrote:
> Add the ability to turn on and off tbkas (traffic based keep alive
> support) via configfs. This is useful for testing and debugging.

Yep, this is indeed useful.

> 
> Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
> ---
>  drivers/nvme/target/admin-cmd.c |  4 +++-
>  drivers/nvme/target/configfs.c  | 27 +++++++++++++++++++++++++++
>  drivers/nvme/target/core.c      |  3 ++-
>  drivers/nvme/target/nvmet.h     |  1 +
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index 7764a3c0195c..e734e4b68784 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -696,7 +696,9 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
>  
>  	/* XXX: figure out what to do about RTD3R/RTD3 */
>  	id->oaes = cpu_to_le32(NVMET_AEN_CFG_OPTIONAL);
> -	ctratt = NVME_CTRL_ATTR_HID_128_BIT | NVME_CTRL_ATTR_TBKAS;
> +	ctratt = NVME_CTRL_ATTR_HID_128_BIT;
> +	if (subsys->tbkas)
> +		ctratt |= NVME_CTRL_ATTR_TBKAS;
>  	if (nvmet_is_pci_ctrl(ctrl))
>  		ctratt |= NVME_CTRL_ATTR_RHII;
>  	id->ctratt = cpu_to_le32(ctratt);
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 6286e38436dd..5766e594e724 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -1369,6 +1369,32 @@ static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
>  }
>  CONFIGFS_ATTR(nvmet_subsys_, attr_serial);
>  
> +static ssize_t nvmet_subsys_attr_tbkas_show(struct config_item *item,
> +					     char *page)
> +{
> +	return snprintf(page, PAGE_SIZE, "%d\n", to_subsys(item)->tbkas);
> +}
> +
> +static ssize_t nvmet_subsys_attr_tbkas_store(struct config_item *item,
> +					      const char *page, size_t count)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	bool tbkas;
> +
> +	if (subsys->subsys_discovered) {
> +		pr_err("Can't set traffic based keep alive support. %d is already assigned\n",
> +		       subsys->tbkas);
> +		return -EINVAL;
> +	}

Would it be okay to fore reconnecting existing controllers when
subsys->tbkas changes? Similar to what attr_qid_max does?

I think this will make testing more flexible by avoiding need to
recreate the subsystem in order to test tkbas on/off. Also, you may
consider adding ctrl->tkbas, again just like qid_max. It will be copied
from subsys in nvmet_alloc_ctrl() under subsys->lock.

> +
> +	if (kstrtobool(page, &tbkas))
> +		return -EINVAL;
> +
> +	subsys->tbkas = tbkas;
> +	return count;
> +}
> +CONFIGFS_ATTR(nvmet_subsys_, attr_tbkas);
> +
>  static ssize_t nvmet_subsys_attr_cntlid_min_show(struct config_item *item,
>  						 char *page)
>  {
> @@ -1706,6 +1732,7 @@ static struct configfs_attribute *nvmet_subsys_attrs[] = {
>  	&nvmet_subsys_attr_attr_qid_max,
>  	&nvmet_subsys_attr_attr_ieee_oui,
>  	&nvmet_subsys_attr_attr_firmware,
> +	&nvmet_subsys_attr_attr_tbkas,
>  #ifdef CONFIG_BLK_DEV_INTEGRITY
>  	&nvmet_subsys_attr_attr_pi_enable,
>  #endif
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 43871a8f56ca..20aeb9624be0 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1227,7 +1227,7 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_sq *sq,
>  		goto fail;
>  	}
>  
> -	if (sq->ctrl)
> +	if (sq->ctrl && sq->ctrl->subsys->tbkas)
>  		sq->ctrl->reset_tbkas = true;

If you add ctrl->tkbas, then this will be sq->ctrl->tkbas. It is
guaranteed not to change during the lifetime of the controller.

>  
>  	return true;
> @@ -1865,6 +1865,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>  	}
>  
>  	subsys->ieee_oui = 0;
> +	subsys->tbkas = true;
>  
>  	subsys->firmware_rev = kstrndup(UTS_RELEASE, NVMET_FR_MAX_SIZE, GFP_KERNEL);
>  	if (!subsys->firmware_rev) {
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index dbda55895f4f..241eac4fa969 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -343,6 +343,7 @@ struct nvmet_subsys {
>  	bool			subsys_discovered;
>  	char			*subsysnqn;
>  	bool			pi_support;
> +	bool			tbkas;
>  
>  	struct config_group	group;
>  
> -- 
> 2.55.0
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-22 22:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 20:22 [PATCH v1] nvmet: Add traffic based keep alive support to configfs Jesse Taube
2026-09-22 22:12 ` Mohamed Khalfella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®