[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200902215102.GA1317974@us.ibm.com>
Date: Wed, 2 Sep 2020 14:51:02 -0700
From: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
To: Thomas Falcon <tlfalcon@...ux.ibm.com>
Cc: netdev@...r.kernel.org, drt@...ux.vnet.ibm.com,
ljp@...ux.vnet.ibm.com, cforno12@...ux.ibm.com
Subject: Re: [PATCH net-next 4/5] ibmvnic: Reporting device ACL settings
through sysfs
Thomas Falcon [tlfalcon@...ux.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@...ux.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
Powered by blists - more mailing lists