[PATCH net-next 0/5] ibmvnic: Report ACL settings in sysfs

[PATCH net-next 0/5] ibmvnic: Report ACL settings in sysfs

[Thomas Falcon]

These patches provide support for VNIC Access Control List
settings reporting through sysfs, where they may be exposed
to administrators or tools. ABI Documentation is provided for
existing sysfs device files as well.

Thomas Falcon (5):
  ibmvnic: Create failover sysfs as part of an attribute group
  ibmvnic: Include documentation for ibmvnic sysfs files
  ibmvnic: Remove ACL change indication definitions
  ibmvnic: Reporting device ACL settings through sysfs
  ibmvnic: Provide documentation for ACL sysfs files

 Documentation/ABI/testing/sysfs-driver-ibmvnic |  40 +++++
 drivers/net/ethernet/ibm/ibmvnic.c             | 230 ++++++++++++++++++++++++-
 drivers/net/ethernet/ibm/ibmvnic.h             |  36 ++--
 3 files changed, 284 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-ibmvnic

-- 
1.8.3.1

[PATCH net-next 1/5] ibmvnic: Create failover sysfs as part of an attribute group

[Thomas Falcon]

Create a sysfs attribute group and make failover sysfs file a
member.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 86a83e5..91b9cc3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5047,7 +5047,7 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter, bool reset)
 	return rc;
 }
 
-static struct device_attribute dev_attr_failover;
+static const struct attribute_group *dev_attr_groups[];
 
 static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 {
@@ -5126,9 +5126,9 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	netdev->min_mtu = adapter->min_mtu - ETH_HLEN;
 	netdev->max_mtu = adapter->max_mtu - ETH_HLEN;
 
-	rc = device_create_file(&dev->dev, &dev_attr_failover);
+	rc = sysfs_create_groups(&dev->dev.kobj, dev_attr_groups);
 	if (rc)
-		goto ibmvnic_dev_file_err;
+		goto ibmvnic_dev_groups_err;
 
 	netif_carrier_off(netdev);
 	rc = register_netdev(netdev);
@@ -5145,14 +5145,11 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	return 0;
 
 ibmvnic_register_fail:
-	device_remove_file(&dev->dev, &dev_attr_failover);
-
-ibmvnic_dev_file_err:
+	sysfs_remove_groups(&dev->dev.kobj, dev_attr_groups);
+ibmvnic_dev_groups_err:
 	release_stats_token(adapter);
-
 ibmvnic_stats_fail:
 	release_stats_buffers(adapter);
-
 ibmvnic_init_fail:
 	release_sub_crqs(adapter, 1);
 	release_crq_queue(adapter);
@@ -5194,13 +5191,15 @@ static int ibmvnic_remove(struct vio_dev *dev)
 
 	rtnl_unlock();
 	mutex_destroy(&adapter->fw_lock);
-	device_remove_file(&dev->dev, &dev_attr_failover);
+	sysfs_remove_groups(&dev->dev.kobj, dev_attr_groups);
 	free_netdev(netdev);
 	dev_set_drvdata(&dev->dev, NULL);
 
 	return 0;
 }
 
+static struct device_attribute dev_attr_failover;
+
 static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -5237,6 +5236,20 @@ static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR_WO(failover);
 
+static struct attribute *dev_attrs[] = {
+	&dev_attr_failover.attr,
+	NULL,
+};
+
+static struct attribute_group dev_attr_group = {
+	.attrs = dev_attrs,
+};
+
+static const struct attribute_group *dev_attr_groups[] = {
+	&dev_attr_group,
+	NULL,
+};
+
 static unsigned long ibmvnic_get_desired_dma(struct vio_dev *vdev)
 {
 	struct net_device *netdev = dev_get_drvdata(&vdev->dev);
-- 
1.8.3.1

[PATCH net-next 2/5] ibmvnic: Include documentation for ibmvnic sysfs files

[Thomas Falcon]

Include documentation for existing ibmvnic sysfs files,
currently only for "failover," which is used to swap
the active hardware port to a backup port in redundant
backing hardware or failover configurations.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-driver-ibmvnic | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-ibmvnic

diff --git a/Documentation/ABI/testing/sysfs-driver-ibmvnic b/Documentation/ABI/testing/sysfs-driver-ibmvnic
new file mode 100644
index 0000000..7fa2920
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-ibmvnic
@@ -0,0 +1,14 @@
+What:		/sys/devices/vio/<our device>/failover
+Date:		June 2017
+KernelVersion:	4.13
+Contact:	linuxppc-dev@lists.ozlabs.org
+Description:	If the ibmvnic device has been configured with redundant
+		physical NIC ports, the user may write "1" to the failover
+		file to trigger a device failover, which will reset the
+		ibmvnic device and swap to a backup physical port. If no
+		redundant physical port has been configured for the device,
+		the device will not reset and -EINVAL is returned. If anything
+		other than "1" is written to the file, -EINVAL will also be
+		returned.
+Users:		Any users of the ibmvnic driver which use redundant hardware
+		configurations.
-- 
1.8.3.1

Re: [PATCH net-next 2/5] ibmvnic: Include documentation for ibmvnic sysfs files

[Jakub Kicinski]

On Mon, 31 Aug 2020 11:58:10 -0500 Thomas Falcon wrote:
> Include documentation for existing ibmvnic sysfs files,
> currently only for "failover," which is used to swap
> the active hardware port to a backup port in redundant
> backing hardware or failover configurations.
> 
> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-driver-ibmvnic | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-ibmvnic
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ibmvnic b/Documentation/ABI/testing/sysfs-driver-ibmvnic
> new file mode 100644
> index 0000000..7fa2920
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-ibmvnic
> @@ -0,0 +1,14 @@
> +What:		/sys/devices/vio/<our device>/failover
> +Date:		June 2017
> +KernelVersion:	4.13
> +Contact:	linuxppc-dev@lists.ozlabs.org
> +Description:	If the ibmvnic device has been configured with redundant
> +		physical NIC ports, the user may write "1" to the failover
> +		file to trigger a device failover, which will reset the
> +		ibmvnic device and swap to a backup physical port. If no
> +		redundant physical port has been configured for the device,
> +		the device will not reset and -EINVAL is returned. If anything
> +		other than "1" is written to the file, -EINVAL will also be
> +		returned.
> +Users:		Any users of the ibmvnic driver which use redundant hardware
> +		configurations.

Could you elaborate what the failover thing is? Is it what net_failover
does or something opposite? (you say "backup physical port" which
sounds like physical port is a backup.. perhaps some IBM nomenclature
there worth clarifying?)

Re: [PATCH net-next 2/5] ibmvnic: Include documentation for ibmvnic sysfs files

[Thomas Falcon]

On 8/31/20 2:07 PM, Jakub Kicinski wrote:
> On Mon, 31 Aug 2020 11:58:10 -0500 Thomas Falcon wrote:
>> Include documentation for existing ibmvnic sysfs files,
>> currently only for "failover," which is used to swap
>> the active hardware port to a backup port in redundant
>> backing hardware or failover configurations.
>>
>> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
>> ---
>>   Documentation/ABI/testing/sysfs-driver-ibmvnic | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-ibmvnic
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-ibmvnic b/Documentation/ABI/testing/sysfs-driver-ibmvnic
>> new file mode 100644
>> index 0000000..7fa2920
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-ibmvnic
>> @@ -0,0 +1,14 @@
>> +What:		/sys/devices/vio/<our device>/failover
>> +Date:		June 2017
>> +KernelVersion:	4.13
>> +Contact:	linuxppc-dev@lists.ozlabs.org
>> +Description:	If the ibmvnic device has been configured with redundant
>> +		physical NIC ports, the user may write "1" to the failover
>> +		file to trigger a device failover, which will reset the
>> +		ibmvnic device and swap to a backup physical port. If no
>> +		redundant physical port has been configured for the device,
>> +		the device will not reset and -EINVAL is returned. If anything
>> +		other than "1" is written to the file, -EINVAL will also be
>> +		returned.
>> +Users:		Any users of the ibmvnic driver which use redundant hardware
>> +		configurations.
> Could you elaborate what the failover thing is? Is it what net_failover
> does or something opposite? (you say "backup physical port" which
> sounds like physical port is a backup.. perhaps some IBM nomenclature
> there worth clarifying?)

Hi Jakub,

When creating a SRIOV VNIC device on a Power system, the user will 
specify one or more ports to use from physical NIC's available to the 
Power Hypervisor. These aren't visible to the Linux OS. In a failover 
configuration, the VNIC will have one active port and at least one other 
port in backup or standby mode. It's similar to the bonding driver's 
active-backup mode. If the hypervisor detects a problem with the active 
port, it will swap in the backup port and send a signal to the VNIC 
driver that it should reset, which is needed to activate the new port. 
There is also a mechanism through which the driver can force this 
operation in case the hypervisor does not detect an issue with the 
active port. This mechanism can be triggered by an administrator or with 
userspace tools through the 'failover' device file in sysfs.

Tom

[PATCH net-next 3/5] ibmvnic: Remove ACL change indication definitions

[Thomas Falcon]

Access Control Lists can not be dynamically changed,
so an existing device can never be notified of an update
in ACL settings. Remove it.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 8da9879..e497392 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -576,15 +576,6 @@ struct ibmvnic_get_vpd_rsp {
 	struct ibmvnic_rc rc;
 } __packed __aligned(8);
 
-struct ibmvnic_acl_change_indication {
-	u8 first;
-	u8 cmd;
-	__be16 change_type;
-#define IBMVNIC_MAC_ACL 0
-#define IBMVNIC_VLAN_ACL 1
-	u8 reserved[12];
-} __packed __aligned(8);
-
 struct ibmvnic_acl_query {
 	u8 first;
 	u8 cmd;
@@ -703,7 +694,6 @@ struct ibmvnic_query_map_rsp {
 	struct ibmvnic_get_vpd_size_rsp get_vpd_size_rsp;
 	struct ibmvnic_get_vpd get_vpd;
 	struct ibmvnic_get_vpd_rsp get_vpd_rsp;
-	struct ibmvnic_acl_change_indication acl_change_indication;
 	struct ibmvnic_acl_query acl_query;
 	struct ibmvnic_generic_crq acl_query_rsp;
 	struct ibmvnic_tune tune;
-- 
1.8.3.1

[PATCH net-next 4/5] ibmvnic: Reporting device ACL settings through sysfs

[Thomas Falcon]

Access Control Lists can be defined for each IBM VNIC
adapter at time of creation. MAC address and VLAN ID's
may be specified, as well as a Port VLAN ID (PVID).
These may all be requested though read-only sysfs files:
mac_acl, vlan_acl, and pvid. When these files are read,
a series of Command-Response Queue (CRQ) commands is sent to
firmware. The first command requests the size of the ACL
data. The driver allocates a buffer of this size and passes
the address in a second CRQ command to firmware, which then
writes the ACL data to this buffer. This data is then parsed
and printed to the respective sysfs file.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 199 +++++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/ibm/ibmvnic.h |  26 ++++-
 2 files changed, 222 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 91b9cc3..36dfa69 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1163,6 +1163,60 @@ static int __ibmvnic_open(struct net_device *netdev)
 	return rc;
 }
 
+static int ibmvnic_query_acl_sz(struct ibmvnic_adapter *adapter)
+{
+	union ibmvnic_crq crq;
+
+	memset(&crq, 0, sizeof(crq));
+	crq.acl_query.first = IBMVNIC_CRQ_CMD;
+	crq.acl_query.cmd = ACL_QUERY;
+
+	if (ibmvnic_send_crq(adapter, &crq))
+		return -EIO;
+	return 0;
+}
+
+static int ibmvnic_request_acl_buf(struct ibmvnic_adapter *adapter)
+{
+	struct device *dev = &adapter->vdev->dev;
+	union ibmvnic_crq rcrq;
+	int rc;
+
+	rc = 0;
+	adapter->acl_buf = kmalloc(adapter->acl_buf_sz, GFP_KERNEL);
+	if (!adapter->acl_buf) {
+		rc = -ENOMEM;
+		goto acl_alloc_err;
+	}
+	adapter->acl_buf_token = dma_map_single(dev, adapter->acl_buf,
+						adapter->acl_buf_sz,
+						DMA_FROM_DEVICE);
+	if (dma_mapping_error(dev, adapter->acl_buf_token)) {
+		rc = -ENOMEM;
+		goto acl_dma_err;
+	}
+	memset(&rcrq, 0, sizeof(rcrq));
+	rcrq.acl_query.first = IBMVNIC_CRQ_CMD;
+	rcrq.acl_query.cmd = ACL_QUERY;
+	rcrq.acl_query.ioba = cpu_to_be32(adapter->acl_buf_token);
+	rcrq.acl_query.len = cpu_to_be32(adapter->acl_buf_sz);
+	if (ibmvnic_send_crq(adapter, &rcrq)) {
+		rc = -EIO;
+		goto acl_query_err;
+	}
+	return 0;
+acl_query_err:
+	dma_unmap_single(dev, adapter->acl_buf_token,
+			 adapter->acl_buf_sz, DMA_FROM_DEVICE);
+	adapter->acl_buf_token = 0;
+	adapter->acl_buf_sz = 0;
+acl_dma_err:
+	kfree(adapter->acl_buf);
+	adapter->acl_buf = NULL;
+acl_alloc_err:
+	return rc;
+}
+
 static int ibmvnic_open(struct net_device *netdev)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -4635,6 +4689,25 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
 	return rc;
 }
 
+static void handle_acl_query_rsp(struct ibmvnic_adapter *adapter,
+				 union ibmvnic_crq *crq)
+{
+	struct net_device *netdev = adapter->netdev;
+	u8 rcode;
+
+	rcode = crq->acl_query_rsp.rc.code;
+	adapter->fw_done_rc = rcode;
+	/* NOMEMORY is returned when ACL buffer size request is successful */
+	if (rcode == NOMEMORY) {
+		adapter->acl_buf_sz = be32_to_cpu(crq->acl_query_rsp.len);
+		netdev_dbg(netdev, "ACL buffer size is %d.\n",
+			   adapter->acl_buf_sz);
+	} else if (rcode != SUCCESS) {
+		netdev_err(netdev, "ACL query failed, rc = %u\n", rcode);
+	}
+	complete(&adapter->fw_done);
+}
+
 static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 			       struct ibmvnic_adapter *adapter)
 {
@@ -4798,6 +4871,9 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 		adapter->fw_done_rc = handle_query_phys_parms_rsp(crq, adapter);
 		complete(&adapter->fw_done);
 		break;
+	case ACL_QUERY_RSP:
+		handle_acl_query_rsp(adapter, crq);
+		break;
 	default:
 		netdev_err(netdev, "Got an invalid cmd type 0x%02x\n",
 			   gen_crq->cmd);
@@ -5199,6 +5275,9 @@ static int ibmvnic_remove(struct vio_dev *dev)
 }
 
 static struct device_attribute dev_attr_failover;
+static struct device_attribute dev_attr_vlan_acl;
+static struct device_attribute dev_attr_mac_acl;
+static struct device_attribute dev_attr_pvid;
 
 static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
@@ -5234,10 +5313,130 @@ static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static int ibmvnic_get_acls(struct ibmvnic_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+	int rc;
+
+	mutex_lock(&adapter->fw_lock);
+	reinit_completion(&adapter->fw_done);
+	adapter->fw_done_rc = 0;
+	rc = ibmvnic_query_acl_sz(adapter);
+	if (rc) {
+		netdev_err(netdev, "Query ACL buffer size failed, rc = %d\n",
+			   rc);
+		goto out;
+	}
+	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
+	if (rc) {
+		netdev_err(netdev,
+			   "Query ACL buffer size did not complete, rc = %d\n",
+			   rc);
+		goto out;
+	}
+	/* NOMEMORY is returned when the ACL buffer size is retrieved
+	 * successfully
+	 */
+	if (adapter->fw_done_rc != NOMEMORY) {
+		netdev_err(netdev, "Unable to get ACL buffer size, rc = %d\n",
+			   adapter->fw_done_rc);
+		rc = -EIO;
+		goto out;
+	}
+	reinit_completion(&adapter->fw_done);
+	rc = ibmvnic_request_acl_buf(adapter);
+	if (rc) {
+		netdev_err(netdev, "ACL buffer request failed, rc = %d\n", rc);
+		goto out;
+	}
+	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
+	if (rc) {
+		netdev_err(netdev,
+			   "ACL buffer request did not complete, rc = %d\n",
+			   rc);
+		goto out;
+	}
+	if (adapter->fw_done_rc != SUCCESS) {
+		netdev_err(netdev, "Unable to retrieve ACL buffer, rc = %d\n",
+			   adapter->fw_done_rc);
+		rc = -EIO;
+	}
+out:
+	mutex_unlock(&adapter->fw_lock);
+	return rc;
+}
+
+static ssize_t acl_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct ibmvnic_acl_buffer *acl_buf;
+	struct ibmvnic_adapter *adapter;
+	struct net_device *netdev;
+	int num_entries;
+	ssize_t rsize;
+	int offset;
+	int rc;
+	int i;
+
+	rsize = 0;
+	netdev = dev_get_drvdata(dev);
+	adapter = netdev_priv(netdev);
+	rc = ibmvnic_get_acls(adapter);
+	if (rc)
+		return rc;
+	acl_buf = adapter->acl_buf;
+	if (attr == &dev_attr_mac_acl) {
+		offset = be32_to_cpu(acl_buf->offset_mac_addrs);
+		num_entries = be32_to_cpu(acl_buf->num_mac_addrs);
+		if (num_entries == 0)
+			goto out;
+		for (i = 0; i < num_entries; i++) {
+			char *entry = (char *)acl_buf + offset + i * 6;
+
+			rsize += scnprintf(buf + rsize, PAGE_SIZE,
+					   "%pM\n", entry);
+		}
+	} else if (attr == &dev_attr_vlan_acl) {
+		offset = be32_to_cpu(acl_buf->offset_vlan_ids);
+		num_entries = be32_to_cpu(acl_buf->num_vlan_ids);
+		if (num_entries == 0)
+			goto out;
+		for (i = 0 ; i < num_entries; i++) {
+			char *entry = (char *)acl_buf + offset + i * 2;
+
+			rsize += scnprintf(buf + rsize, PAGE_SIZE, "%d\n",
+					   be16_to_cpup((__be16 *)entry));
+		}
+	} else if (attr == &dev_attr_pvid) {
+		u16 pvid, vid;
+		u8 pri;
+
+		pvid = be16_to_cpu(acl_buf->pvid);
+		vid = pvid & VLAN_VID_MASK;
+		pri = (pvid & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+
+		rsize = scnprintf(buf, PAGE_SIZE, "%d\n%d\n", vid, pri);
+	}
+out:
+	dma_unmap_single(dev, adapter->acl_buf_token, adapter->acl_buf_sz,
+			 DMA_FROM_DEVICE);
+	kfree(adapter->acl_buf);
+	adapter->acl_buf = NULL;
+	adapter->acl_buf_token = 0;
+	adapter->acl_buf_sz = 0;
+	return rsize;
+}
+
 static DEVICE_ATTR_WO(failover);
+static DEVICE_ATTR(mac_acl, 0444, acl_show, NULL);
+static DEVICE_ATTR(vlan_acl, 0444, acl_show, NULL);
+static DEVICE_ATTR(pvid, 0444, acl_show, NULL);
 
 static struct attribute *dev_attrs[] = {
 	&dev_attr_failover.attr,
+	&dev_attr_mac_acl.attr,
+	&dev_attr_vlan_acl.attr,
+	&dev_attr_pvid.attr,
 	NULL,
 };
 
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index e497392..4768626 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -195,12 +195,15 @@ struct ibmvnic_acl_buffer {
 #define INITIAL_VERSION_IOB 1
 	u8 mac_acls_restrict;
 	u8 vlan_acls_restrict;
-	u8 reserved1[22];
+	__be16 pvid;
+	u8 reserved1[52];
+	__be32 max_mac_addrs;
 	__be32 num_mac_addrs;
 	__be32 offset_mac_addrs;
+	__be32 max_vlan_ids;
 	__be32 num_vlan_ids;
 	__be32 offset_vlan_ids;
-	u8 reserved2[80];
+	u8 reserved2[40];
 } __packed __aligned(8);
 
 /* descriptors have been changed, how should this be defined?  1? 4? */
@@ -585,6 +588,19 @@ struct ibmvnic_acl_query {
 	u8 reserved2[4];
 } __packed __aligned(8);
 
+struct ibmvnic_acl_query_rsp {
+	u8 first;
+	u8 cmd;
+#define ACL_EXISTS      0x8000
+#define VLAN_ACL_ON     0x4000
+#define MAC_ACL_ON      0x2000
+#define PVID_ON	        0x1000
+	__be16 flags;
+	u8 reserved[4];
+	__be32 len;
+	struct ibmvnic_rc rc;
+} __packed __aligned(8);
+
 struct ibmvnic_tune {
 	u8 first;
 	u8 cmd;
@@ -695,7 +711,7 @@ struct ibmvnic_query_map_rsp {
 	struct ibmvnic_get_vpd get_vpd;
 	struct ibmvnic_get_vpd_rsp get_vpd_rsp;
 	struct ibmvnic_acl_query acl_query;
-	struct ibmvnic_generic_crq acl_query_rsp;
+	struct ibmvnic_acl_query_rsp acl_query_rsp;
 	struct ibmvnic_tune tune;
 	struct ibmvnic_generic_crq tune_rsp;
 	struct ibmvnic_request_map request_map;
@@ -1001,6 +1017,10 @@ struct ibmvnic_adapter {
 	dma_addr_t login_rsp_buf_token;
 	int login_rsp_buf_sz;
 
+	struct ibmvnic_acl_buffer *acl_buf;
+	dma_addr_t acl_buf_token;
+	int acl_buf_sz;
+
 	atomic_t running_cap_crqs;
 	bool wait_capability;
 
-- 
1.8.3.1

Re: [PATCH net-next 4/5] ibmvnic: Reporting device ACL settings through sysfs

[Sukadev Bhattiprolu]

Thomas Falcon [tlfalcon@linux.ibm.com] wrote:
> Access Control Lists can be defined for each IBM VNIC
> adapter at time of creation. MAC address and VLAN ID's
> may be specified, as well as a Port VLAN ID (PVID).
> These may all be requested though read-only sysfs files:
> mac_acl, vlan_acl, and pvid. When these files are read,
> a series of Command-Response Queue (CRQ) commands is sent to
> firmware. The first command requests the size of the ACL
> data. The driver allocates a buffer of this size and passes
> the address in a second CRQ command to firmware, which then
> writes the ACL data to this buffer. This data is then parsed
> and printed to the respective sysfs file.
> 
> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 199 +++++++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/ibm/ibmvnic.h |  26 ++++-
>  2 files changed, 222 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 91b9cc3..36dfa69 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1163,6 +1163,60 @@ static int __ibmvnic_open(struct net_device *netdev)
>  	return rc;
>  }
> 
> +static int ibmvnic_query_acl_sz(struct ibmvnic_adapter *adapter)
> +{
> +	union ibmvnic_crq crq;
> +
> +	memset(&crq, 0, sizeof(crq));
> +	crq.acl_query.first = IBMVNIC_CRQ_CMD;
> +	crq.acl_query.cmd = ACL_QUERY;
> +
> +	if (ibmvnic_send_crq(adapter, &crq))
> +		return -EIO;
> +	return 0;
> +}
> +
> +static int ibmvnic_request_acl_buf(struct ibmvnic_adapter *adapter)
> +{
> +	struct device *dev = &adapter->vdev->dev;
> +	union ibmvnic_crq rcrq;
> +	int rc;
> +
> +	rc = 0;
> +	adapter->acl_buf = kmalloc(adapter->acl_buf_sz, GFP_KERNEL);
> +	if (!adapter->acl_buf) {
> +		rc = -ENOMEM;
> +		goto acl_alloc_err;
> +	}
> +	adapter->acl_buf_token = dma_map_single(dev, adapter->acl_buf,
> +						adapter->acl_buf_sz,
> +						DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dev, adapter->acl_buf_token)) {
> +		rc = -ENOMEM;
> +		goto acl_dma_err;
> +	}
> +	memset(&rcrq, 0, sizeof(rcrq));
> +	rcrq.acl_query.first = IBMVNIC_CRQ_CMD;
> +	rcrq.acl_query.cmd = ACL_QUERY;
> +	rcrq.acl_query.ioba = cpu_to_be32(adapter->acl_buf_token);
> +	rcrq.acl_query.len = cpu_to_be32(adapter->acl_buf_sz);
> +	if (ibmvnic_send_crq(adapter, &rcrq)) {
> +		rc = -EIO;
> +		goto acl_query_err;
> +	}
> +	return 0;
> +acl_query_err:
> +	dma_unmap_single(dev, adapter->acl_buf_token,
> +			 adapter->acl_buf_sz, DMA_FROM_DEVICE);
> +	adapter->acl_buf_token = 0;
> +	adapter->acl_buf_sz = 0;
> +acl_dma_err:
> +	kfree(adapter->acl_buf);
> +	adapter->acl_buf = NULL;
> +acl_alloc_err:
> +	return rc;
> +}
> +
>  static int ibmvnic_open(struct net_device *netdev)
>  {
>  	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
> @@ -4635,6 +4689,25 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
>  	return rc;
>  }
> 
> +static void handle_acl_query_rsp(struct ibmvnic_adapter *adapter,
> +				 union ibmvnic_crq *crq)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +	u8 rcode;
> +
> +	rcode = crq->acl_query_rsp.rc.code;
> +	adapter->fw_done_rc = rcode;
> +	/* NOMEMORY is returned when ACL buffer size request is successful */
> +	if (rcode == NOMEMORY) {
> +		adapter->acl_buf_sz = be32_to_cpu(crq->acl_query_rsp.len);
> +		netdev_dbg(netdev, "ACL buffer size is %d.\n",
> +			   adapter->acl_buf_sz);
> +	} else if (rcode != SUCCESS) {
> +		netdev_err(netdev, "ACL query failed, rc = %u\n", rcode);
> +	}
> +	complete(&adapter->fw_done);
> +}
> +
>  static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
>  			       struct ibmvnic_adapter *adapter)
>  {
> @@ -4798,6 +4871,9 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
>  		adapter->fw_done_rc = handle_query_phys_parms_rsp(crq, adapter);
>  		complete(&adapter->fw_done);
>  		break;
> +	case ACL_QUERY_RSP:
> +		handle_acl_query_rsp(adapter, crq);
> +		break;
>  	default:
>  		netdev_err(netdev, "Got an invalid cmd type 0x%02x\n",
>  			   gen_crq->cmd);
> @@ -5199,6 +5275,9 @@ static int ibmvnic_remove(struct vio_dev *dev)
>  }
> 
>  static struct device_attribute dev_attr_failover;
> +static struct device_attribute dev_attr_vlan_acl;
> +static struct device_attribute dev_attr_mac_acl;
> +static struct device_attribute dev_attr_pvid;
> 
>  static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
>  			      const char *buf, size_t count)
> @@ -5234,10 +5313,130 @@ static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
> 
> +static int ibmvnic_get_acls(struct ibmvnic_adapter *adapter)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +	int rc;
> +
> +	mutex_lock(&adapter->fw_lock);

Would it be better to hold this lock in the caller acl_show() instead?
If we get back to back calls to acl_show(), the thread in acl_show()
could free the adapter->acl_buf while this function is still using?

> +	reinit_completion(&adapter->fw_done);
> +	adapter->fw_done_rc = 0;
> +	rc = ibmvnic_query_acl_sz(adapter);
> +	if (rc) {
> +		netdev_err(netdev, "Query ACL buffer size failed, rc = %d\n",
> +			   rc);
> +		goto out;
> +	}
> +	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
> +	if (rc) {
> +		netdev_err(netdev,
> +			   "Query ACL buffer size did not complete, rc = %d\n",
> +			   rc);
> +		goto out;
> +	}
> +	/* NOMEMORY is returned when the ACL buffer size is retrieved
> +	 * successfully
> +	 */
> +	if (adapter->fw_done_rc != NOMEMORY) {
> +		netdev_err(netdev, "Unable to get ACL buffer size, rc = %d\n",
> +			   adapter->fw_done_rc);
> +		rc = -EIO;
> +		goto out;
> +	}
> +	reinit_completion(&adapter->fw_done);
> +	rc = ibmvnic_request_acl_buf(adapter);
> +	if (rc) {
> +		netdev_err(netdev, "ACL buffer request failed, rc = %d\n", rc);
> +		goto out;
> +	}
> +	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
> +	if (rc) {
> +		netdev_err(netdev,
> +			   "ACL buffer request did not complete, rc = %d\n",
> +			   rc);
> +		goto out;
> +	}
> +	if (adapter->fw_done_rc != SUCCESS) {
> +		netdev_err(netdev, "Unable to retrieve ACL buffer, rc = %d\n",
> +			   adapter->fw_done_rc);
> +		rc = -EIO;
> +	}
> +out:
> +	mutex_unlock(&adapter->fw_lock);
> +	return rc;
> +}
> +
> +static ssize_t acl_show(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct ibmvnic_acl_buffer *acl_buf;
> +	struct ibmvnic_adapter *adapter;
> +	struct net_device *netdev;
> +	int num_entries;
> +	ssize_t rsize;
> +	int offset;
> +	int rc;
> +	int i;
> +
> +	rsize = 0;
> +	netdev = dev_get_drvdata(dev);
> +	adapter = netdev_priv(netdev);
> +	rc = ibmvnic_get_acls(adapter);
> +	if (rc)
> +		return rc;
> +	acl_buf = adapter->acl_buf;
> +	if (attr == &dev_attr_mac_acl) {
> +		offset = be32_to_cpu(acl_buf->offset_mac_addrs);
> +		num_entries = be32_to_cpu(acl_buf->num_mac_addrs);
> +		if (num_entries == 0)
> +			goto out;
> +		for (i = 0; i < num_entries; i++) {
> +			char *entry = (char *)acl_buf + offset + i * 6;
> +
> +			rsize += scnprintf(buf + rsize, PAGE_SIZE,
> +					   "%pM\n", entry);

Shouldn't the second parameter be 'PAGE_SIZE-rsize' here and

> +		}
> +	} else if (attr == &dev_attr_vlan_acl) {
> +		offset = be32_to_cpu(acl_buf->offset_vlan_ids);
> +		num_entries = be32_to_cpu(acl_buf->num_vlan_ids);
> +		if (num_entries == 0)
> +			goto out;
> +		for (i = 0 ; i < num_entries; i++) {
> +			char *entry = (char *)acl_buf + offset + i * 2;
> +
> +			rsize += scnprintf(buf + rsize, PAGE_SIZE, "%d\n",
> +					   be16_to_cpup((__be16 *)entry));

here?

> +		}
> +	} else if (attr == &dev_attr_pvid) {
> +		u16 pvid, vid;
> +		u8 pri;
> +
> +		pvid = be16_to_cpu(acl_buf->pvid);
> +		vid = pvid & VLAN_VID_MASK;
> +		pri = (pvid & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
> +
> +		rsize = scnprintf(buf, PAGE_SIZE, "%d\n%d\n", vid, pri);
> +	}
> +out:
> +	dma_unmap_single(dev, adapter->acl_buf_token, adapter->acl_buf_sz,
> +			 DMA_FROM_DEVICE);
> +	kfree(adapter->acl_buf);
> +	adapter->acl_buf = NULL;
> +	adapter->acl_buf_token = 0;
> +	adapter->acl_buf_sz = 0;
> +	return rsize;
> +}
> +
>  static DEVICE_ATTR_WO(failover);
> +static DEVICE_ATTR(mac_acl, 0444, acl_show, NULL);
> +static DEVICE_ATTR(vlan_acl, 0444, acl_show, NULL);
> +static DEVICE_ATTR(pvid, 0444, acl_show, NULL);
> 
>  static struct attribute *dev_attrs[] = {
>  	&dev_attr_failover.attr,
> +	&dev_attr_mac_acl.attr,
> +	&dev_attr_vlan_acl.attr,
> +	&dev_attr_pvid.attr,
>  	NULL,
>  };
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
> index e497392..4768626 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.h
> +++ b/drivers/net/ethernet/ibm/ibmvnic.h
> @@ -195,12 +195,15 @@ struct ibmvnic_acl_buffer {
>  #define INITIAL_VERSION_IOB 1
>  	u8 mac_acls_restrict;
>  	u8 vlan_acls_restrict;
> -	u8 reserved1[22];
> +	__be16 pvid;
> +	u8 reserved1[52];
> +	__be32 max_mac_addrs;
>  	__be32 num_mac_addrs;
>  	__be32 offset_mac_addrs;
> +	__be32 max_vlan_ids;
>  	__be32 num_vlan_ids;
>  	__be32 offset_vlan_ids;
> -	u8 reserved2[80];
> +	u8 reserved2[40];
>  } __packed __aligned(8);
> 
>  /* descriptors have been changed, how should this be defined?  1? 4? */
> @@ -585,6 +588,19 @@ struct ibmvnic_acl_query {
>  	u8 reserved2[4];
>  } __packed __aligned(8);
> 
> +struct ibmvnic_acl_query_rsp {
> +	u8 first;
> +	u8 cmd;
> +#define ACL_EXISTS      0x8000
> +#define VLAN_ACL_ON     0x4000
> +#define MAC_ACL_ON      0x2000
> +#define PVID_ON	        0x1000
> +	__be16 flags;
> +	u8 reserved[4];
> +	__be32 len;
> +	struct ibmvnic_rc rc;
> +} __packed __aligned(8);
> +
>  struct ibmvnic_tune {
>  	u8 first;
>  	u8 cmd;
> @@ -695,7 +711,7 @@ struct ibmvnic_query_map_rsp {
>  	struct ibmvnic_get_vpd get_vpd;
>  	struct ibmvnic_get_vpd_rsp get_vpd_rsp;
>  	struct ibmvnic_acl_query acl_query;
> -	struct ibmvnic_generic_crq acl_query_rsp;
> +	struct ibmvnic_acl_query_rsp acl_query_rsp;
>  	struct ibmvnic_tune tune;
>  	struct ibmvnic_generic_crq tune_rsp;
>  	struct ibmvnic_request_map request_map;
> @@ -1001,6 +1017,10 @@ struct ibmvnic_adapter {
>  	dma_addr_t login_rsp_buf_token;
>  	int login_rsp_buf_sz;
> 
> +	struct ibmvnic_acl_buffer *acl_buf;
> +	dma_addr_t acl_buf_token;
> +	int acl_buf_sz;
> +
>  	atomic_t running_cap_crqs;
>  	bool wait_capability;
> 
> -- 
> 1.8.3.1

[PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Thomas Falcon]

Provide documentation for ibmvnic device Access Control List
files.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-driver-ibmvnic | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ibmvnic b/Documentation/ABI/testing/sysfs-driver-ibmvnic
index 7fa2920..8c78312 100644
--- a/Documentation/ABI/testing/sysfs-driver-ibmvnic
+++ b/Documentation/ABI/testing/sysfs-driver-ibmvnic
@@ -12,3 +12,29 @@ Description:	If the ibmvnic device has been configured with redundant
 		returned.
 Users:		Any users of the ibmvnic driver which use redundant hardware
 		configurations.
+
+What:		/sys/devices/vio/<our device>/mac_acls
+Date:		August 2020
+KernelVersion:	5.10
+Contact:	linuxppc-dev@lists.ozlabs.org
+Description:	Read-only file which lists the current entries in the ibmvnic
+		device's MAC address Access Control List. Each entry is
+		separated by a new line.
+Users:		Any users of the ibmvnic driver
+
+What:		/sys/devices/vio/<our device>/vlan_acls
+Date:		August 2020
+KernelVersion:	5.10
+Contact:	linuxppc-dev@lists.ozlabs.org
+Description:	Read-only file which lists the current entries in the ibmvnic
+		device's VLAN ID Access Control List. Each entry is separated
+		by a new line.
+Users:		Any users of the ibmvnic driver
+
+What:		/sys/devices/vio/<our device>/pvid
+Date:		August 2020
+KernelVersion:	5.10
+Contact:	linuxppc-dev@lists.ozlabs.org
+Description:	Read-only file which lists the ibmvnic device's Port VLAN
+		ID and Priority setting. Each entry is separated by a new line.
+Users:		Any users of the ibmvnic driver
-- 
1.8.3.1

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Jakub Kicinski]

On Mon, 31 Aug 2020 11:58:13 -0500 Thomas Falcon wrote:
> Provide documentation for ibmvnic device Access Control List
> files.

What API is used to set those parameters in the first place?

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Thomas Falcon]

On 8/31/20 2:26 PM, Jakub Kicinski wrote:
> On Mon, 31 Aug 2020 11:58:13 -0500 Thomas Falcon wrote:
>> Provide documentation for ibmvnic device Access Control List
>> files.
> What API is used to set those parameters in the first place?
>
These parameters are specified in the system's IBM Hardware Management 
Console (HMC) when the VNIC device is create.

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Jakub Kicinski]

On Mon, 31 Aug 2020 14:54:06 -0500 Thomas Falcon wrote:
> On 8/31/20 2:26 PM, Jakub Kicinski wrote:
> > On Mon, 31 Aug 2020 11:58:13 -0500 Thomas Falcon wrote:  
> >> Provide documentation for ibmvnic device Access Control List
> >> files.  
> > What API is used to set those parameters in the first place?
> >  
> These parameters are specified in the system's IBM Hardware Management 
> Console (HMC) when the VNIC device is create.

The new attributes are visible in the "guest" OS, correct?

This seems similar to normal SR-IOV operation, but I've not heard of
use cases for them VM to know what its pvid is. Could you elaborate?

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Thomas Falcon]

On 8/31/20 3:11 PM, Jakub Kicinski wrote:
> On Mon, 31 Aug 2020 14:54:06 -0500 Thomas Falcon wrote:
>> On 8/31/20 2:26 PM, Jakub Kicinski wrote:
>>> On Mon, 31 Aug 2020 11:58:13 -0500 Thomas Falcon wrote:
>>>> Provide documentation for ibmvnic device Access Control List
>>>> files.
>>> What API is used to set those parameters in the first place?
>>>   
>> These parameters are specified in the system's IBM Hardware Management
>> Console (HMC) when the VNIC device is create.
> The new attributes are visible in the "guest" OS, correct?

Correct.

>
> This seems similar to normal SR-IOV operation, but I've not heard of
> use cases for them VM to know what its pvid is. Could you elaborate?
It's provided for informational purposes.

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Jakub Kicinski]

On Mon, 31 Aug 2020 16:44:06 -0500 Thomas Falcon wrote:
> On 8/31/20 3:11 PM, Jakub Kicinski wrote:
> > This seems similar to normal SR-IOV operation, but I've not heard of
> > use cases for them VM to know what its pvid is. Could you elaborate?  
> It's provided for informational purposes.

Seems like an information leak :S and since it's equivalent to the
standard SR-IOV functionality - we'd strongly prefer a common
interface for all use cases. sysfs won't be it. Jiri & Mellanox had 
been working on something in devlink for quite some time.

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[David Miller]

From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 31 Aug 2020 15:00:50 -0700

> On Mon, 31 Aug 2020 16:44:06 -0500 Thomas Falcon wrote:
>> On 8/31/20 3:11 PM, Jakub Kicinski wrote:
>> > This seems similar to normal SR-IOV operation, but I've not heard of
>> > use cases for them VM to know what its pvid is. Could you elaborate?  
>> It's provided for informational purposes.
> 
> Seems like an information leak :S and since it's equivalent to the
> standard SR-IOV functionality - we'd strongly prefer a common
> interface for all use cases. sysfs won't be it. Jiri & Mellanox had 
> been working on something in devlink for quite some time.

Agreed, Thomas please work with Jiri et al. so that you can provide
this information using a standard facility.

Thanks.

Re: [PATCH net-next 5/5] ibmvnic: Provide documentation for ACL sysfs files

[Thomas Falcon]

On 8/31/20 5:17 PM, David Miller wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Mon, 31 Aug 2020 15:00:50 -0700
>
>> On Mon, 31 Aug 2020 16:44:06 -0500 Thomas Falcon wrote:
>>> On 8/31/20 3:11 PM, Jakub Kicinski wrote:
>>>> This seems similar to normal SR-IOV operation, but I've not heard of
>>>> use cases for them VM to know what its pvid is. Could you elaborate?
>>> It's provided for informational purposes.
>> Seems like an information leak :S and since it's equivalent to the
>> standard SR-IOV functionality - we'd strongly prefer a common
>> interface for all use cases. sysfs won't be it. Jiri & Mellanox had
>> been working on something in devlink for quite some time.
> Agreed, Thomas please work with Jiri et al. so that you can provide
> this information using a standard facility.
>
> Thanks.

Thank you for the feedback. I will look into that.