git: 0e7d31f63b9d - main - ena: hide sysctl nodes for unused ENA queues

[Marcin Wojtas]

The branch main has been updated by mw:

URL: https://cgit.FreeBSD.org/src/commit/?id=0e7d31f63b9db869c91228d8ed1e984bdee2b931

commit 0e7d31f63b9db869c91228d8ed1e984bdee2b931
Author:     Marcin Wojtas <mw at FreeBSD.org>
AuthorDate: 2021-06-14 08:57:54 +0000
Commit:     Marcin Wojtas <mw at FreeBSD.org>
CommitDate: 2021-06-24 14:02:39 +0000

    ena: hide sysctl nodes for unused ENA queues
    
    IO queue related attributes are registered statically at driver attach
    with the rest of the ENA specific sysctl nodes. However, the number of
    queues can be changed at runtime via the `ena_sysctl_io_queues_nb`
    request, leading to a potential exposure of attributes for non-existing
    queues.
    
    Introduce a new `ena_sysctl_update_queue_node_nb` function, which
    updates the sysctl nodes after the number of queues is altered.
    This happens by either registering or unregistering node specific oids,
    based on a delta between the previous and current queue count.
    
    NOTE: All unregistered oids must be registered again before the driver
    detach, e.g. by another call to this function.
    
    Submitted by: Artur Rojek <ar at semihalf.com>
    Obtained from: Semihalf
    MFC after: 2 weeks
    Sponsored by: Amazon, Inc.
---
 sys/dev/ena/ena.c        |  4 ++++
 sys/dev/ena/ena.h        |  1 +
 sys/dev/ena/ena_sysctl.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 sys/dev/ena/ena_sysctl.h |  2 ++
 4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c
index d7a68e94d361..fbe32a7a930e 100644
--- a/sys/dev/ena/ena.c
+++ b/sys/dev/ena/ena.c
@@ -3830,6 +3830,10 @@ ena_detach(device_t pdev)
 	ena_destroy_device(adapter, true);
 	ENA_LOCK_UNLOCK(adapter);
 
+	/* Restore unregistered sysctl queue nodes. */
+	ena_sysctl_update_queue_node_nb(adapter, adapter->num_io_queues,
+	    adapter->max_num_io_queues);
+
 #ifdef DEV_NETMAP
 	netmap_detach(adapter->ifp);
 #endif /* DEV_NETMAP */
diff --git a/sys/dev/ena/ena.h b/sys/dev/ena/ena.h
index f3e92f31341a..a49ac70761ba 100644
--- a/sys/dev/ena/ena.h
+++ b/sys/dev/ena/ena.h
@@ -215,6 +215,7 @@ struct ena_que {
 
 	uint32_t id;
 	int cpu;
+	struct sysctl_oid *oid;
 };
 
 struct ena_calc_queue_size_ctx {
diff --git a/sys/dev/ena/ena_sysctl.c b/sys/dev/ena/ena_sysctl.c
index 26fc79379e33..0726771e5adc 100644
--- a/sys/dev/ena/ena_sysctl.c
+++ b/sys/dev/ena/ena_sysctl.c
@@ -28,6 +28,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 #include <sys/cdefs.h>
+#include <sys/param.h>
 __FBSDID("$FreeBSD$");
 
 #include "ena_sysctl.h"
@@ -188,6 +189,8 @@ ena_sysctl_add_stats(struct ena_adapter *adapter)
 		    namebuf, CTLFLAG_RD | CTLFLAG_MPSAFE, NULL, "Queue Name");
 		queue_list = SYSCTL_CHILDREN(queue_node);
 
+		adapter->que[i].oid = queue_node;
+
 		/* TX specific stats */
 		tx_node = SYSCTL_ADD_NODE(ctx, queue_list, OID_AUTO,
 		    "tx_ring", CTLFLAG_RD | CTLFLAG_MPSAFE, NULL, "TX ring");
@@ -396,6 +399,42 @@ ena_sysctl_add_tuneables(struct ena_adapter *adapter)
 }
 
 
+/*
+ * ena_sysctl_update_queue_node_nb - Register/unregister sysctl queue nodes.
+ *
+ * Whether the nodes are registered or unregistered depends on a delta between
+ * the `old` and `new` parameters, representing the number of queues.
+ *
+ * This function is used to hide sysctl attributes for queue nodes which aren't
+ * currently used by the HW (e.g. after a call to `ena_sysctl_io_queues_nb`).
+ *
+ * NOTE:
+ * All unregistered nodes must be registered again at detach, i.e. by a call to
+ * this function.
+ */
+void
+ena_sysctl_update_queue_node_nb(struct ena_adapter *adapter, int old, int new)
+{
+	device_t dev;
+	struct sysctl_oid *oid;
+	int min, max, i;
+
+	dev = adapter->pdev;
+	min = MIN(old, new);
+	max = MIN(MAX(old, new), adapter->max_num_io_queues);
+
+	for (i = min; i < max; ++i) {
+		oid = adapter->que[i].oid;
+
+		sysctl_wlock();
+		if (old > new)
+			sysctl_unregister_oid(oid);
+		else
+			sysctl_register_oid(oid);
+		sysctl_wunlock();
+	}
+}
+
 static int
 ena_sysctl_buf_ring_size(SYSCTL_HANDLER_ARGS)
 {
@@ -488,7 +527,7 @@ static int
 ena_sysctl_io_queues_nb(SYSCTL_HANDLER_ARGS)
 {
 	struct ena_adapter *adapter = arg1;
-	uint32_t tmp = 0;
+	uint32_t old_num_queues, tmp = 0;
 	int error;
 
 	error = sysctl_wire_old_buffer(req, sizeof(tmp));
@@ -527,7 +566,12 @@ ena_sysctl_io_queues_nb(SYSCTL_HANDLER_ARGS)
 		    "Requested new number of IO queues: %u, current value: "
 		    "%u\n", tmp, adapter->num_io_queues);
 
+		old_num_queues = adapter->num_io_queues;
 		error = ena_update_io_queue_nb(adapter, tmp);
+		if (error != 0)
+			return (error);
+
+		ena_sysctl_update_queue_node_nb(adapter, old_num_queues, tmp);
 	}
 
 	return (error);
diff --git a/sys/dev/ena/ena_sysctl.h b/sys/dev/ena/ena_sysctl.h
index 360e32c3ad18..c6059c9cba36 100644
--- a/sys/dev/ena/ena_sysctl.h
+++ b/sys/dev/ena/ena_sysctl.h
@@ -40,6 +40,8 @@
 #include "ena.h"
 
 void	ena_sysctl_add_nodes(struct ena_adapter *adapter);
+void	ena_sysctl_update_queue_node_nb(struct ena_adapter *adapter, int old,
+    int new);
 
 extern int ena_enable_9k_mbufs;
 #define ena_mbuf_sz (ena_enable_9k_mbufs ? MJUM9BYTES : MJUMPAGESIZE)