Re: [PATCH net-next v3 12/15] ibmveth: Expose per-queue buffer pool details via sysfs
From: Simon Horman <horms@kernel.org>
Date: 2026-07-14 13:01:56
Also in:
linuxppc-dev
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- ibmveth: Expose per-queue buffer pool details via sysfs This commit adds a read-only sysfs attribute buffer_pools to the VIO device to expose per-queue buffer pool details dynamically. It outputs runtime per-queue buffer pressure metrics like size, active, and available buffers.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c index 4e3f49b6346fd..ecc472ee8f71f 100644 --- a/drivers/net/ethernet/ibm/ibmveth.c +++ b/drivers/net/ethernet/ibm/ibmveth.c@@ -2896,6 +2896,52 @@ static const struct net_device_ops ibmveth_netdev_ops = {
[ ... ]
+static ssize_t buffer_pools_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct net_device *netdev = dev_get_drvdata(dev);
+ struct ibmveth_adapter *adapter = netdev_priv(netdev);
+ int len = 0;
+ int i, j;
+
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "Queue Pool Size BuffSize Active Available\n");
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "----- ---- ---- -------- ------ ---------\n");[Severity: Medium] Does this buffer_pools_show() implementation violate the sysfs "one value per file" design rule? The attribute appears to dump a complex, multi-line ASCII table with headers and columns rather than exposing a single value per sysfs file. [ ... ]
quoted hunk ↗ jump to hunk
@@ -3056,6 +3102,14 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id) netdev_dbg(netdev, "registered\n"); + rc = sysfs_create_group(&dev->dev.kobj, &ibmveth_attr_group); + if (rc) { + netdev_err(netdev, "failed to create sysfs attributes rc=%d\n", rc); + unregister_netdev(netdev); + free_netdev(netdev); + return rc; + }
[Severity: High] If userspace brings up the interface immediately after register_netdev() succeeds, could adapter->work be scheduled before sysfs_create_group() fails? If this race occurs, unregister_netdev() does not synchronously cancel the work. Could the global workqueue execute the pending adapter->work item on freed memory after free_netdev() is called in this ibmveth_probe() error path? [Severity: High] This is a pre-existing issue in older error paths of ibmveth_probe(), but it is duplicated here: does this error path leak the adapter->rx_buff_pool kobjects? Since kobject_put() is not called on the pool kobjects before free_netdev() is executed, the kobjects remain attached to the VIO device with dangling pointers to the freed adapter memory. Could subsequent sysfs accesses to those pool kobjects trigger a use-after-free?