From 15e54faa5d5e4840974de7fb5cd737c2179a309a Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Mon, 4 Dec 2023 13:09:32 -0800 Subject: [PATCH 1/5] ionic: Use cached VF attributes Each time a VF attribute is set via iproute a call to get the VF configuration is also made. This is currently problematic because for each VF configuration call there are multiple commands sent to the device. Unfortunately, this doesn't scale well. Fix this by reporting the cached VF attributes. The original change to query the device for getting the VF attributes f16f5be31009 ("ionic: Query FW when getting VF info via ndo_get_vf_config") was made to remain consistent with device set VF attributes. However, after further investigation there is no need to query the device. Signed-off-by: Brett Creeley Signed-off-by: Shannon Nelson Reviewed-by: Florian Fainelli Reviewed-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20231204210936.16587-2-shannon.nelson@amd.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/pensando/ionic/ionic.h | 2 - .../net/ethernet/pensando/ionic/ionic_dev.c | 40 -------- .../net/ethernet/pensando/ionic/ionic_dev.h | 3 +- .../net/ethernet/pensando/ionic/ionic_lif.c | 91 ++----------------- .../net/ethernet/pensando/ionic/ionic_main.c | 22 ----- 5 files changed, 10 insertions(+), 148 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h index 2453a40f6ee8..9ffef2e06885 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic.h +++ b/drivers/net/ethernet/pensando/ionic/ionic.h @@ -91,6 +91,4 @@ int ionic_port_identify(struct ionic *ionic); int ionic_port_init(struct ionic *ionic); int ionic_port_reset(struct ionic *ionic); -const char *ionic_vf_attr_to_str(enum ionic_vf_attr attr); - #endif /* _IONIC_H_ */ diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c index c06576f43916..bb9245d933e4 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c @@ -469,46 +469,6 @@ int ionic_set_vf_config(struct ionic *ionic, int vf, return err; } -int ionic_dev_cmd_vf_getattr(struct ionic *ionic, int vf, u8 attr, - struct ionic_vf_getattr_comp *comp) -{ - union ionic_dev_cmd cmd = { - .vf_getattr.opcode = IONIC_CMD_VF_GETATTR, - .vf_getattr.attr = attr, - .vf_getattr.vf_index = cpu_to_le16(vf), - }; - int err; - - if (vf >= ionic->num_vfs) - return -EINVAL; - - switch (attr) { - case IONIC_VF_ATTR_SPOOFCHK: - case IONIC_VF_ATTR_TRUST: - case IONIC_VF_ATTR_LINKSTATE: - case IONIC_VF_ATTR_MAC: - case IONIC_VF_ATTR_VLAN: - case IONIC_VF_ATTR_RATE: - break; - case IONIC_VF_ATTR_STATSADDR: - default: - return -EINVAL; - } - - mutex_lock(&ionic->dev_cmd_lock); - ionic_dev_cmd_go(&ionic->idev, &cmd); - err = ionic_dev_cmd_wait_nomsg(ionic, DEVCMD_TIMEOUT); - memcpy_fromio(comp, &ionic->idev.dev_cmd_regs->comp.vf_getattr, - sizeof(*comp)); - mutex_unlock(&ionic->dev_cmd_lock); - - if (err && comp->status != IONIC_RC_ENOSUPP) - ionic_dev_cmd_dev_err_print(ionic, cmd.vf_getattr.opcode, - comp->status, err); - - return err; -} - void ionic_vf_start(struct ionic *ionic) { union ionic_dev_cmd cmd = { diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h index 1dbc3cb50b1d..19edcb42d9fd 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h @@ -341,8 +341,7 @@ void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type); int ionic_set_vf_config(struct ionic *ionic, int vf, struct ionic_vf_setattr_cmd *vfc); -int ionic_dev_cmd_vf_getattr(struct ionic *ionic, int vf, u8 attr, - struct ionic_vf_getattr_comp *comp); + void ionic_dev_cmd_queue_identify(struct ionic_dev *idev, u16 lif_type, u8 qtype, u8 qver); void ionic_vf_start(struct ionic *ionic); diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index edc14730ce88..afb77e2d04c5 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -2332,82 +2332,11 @@ static int ionic_eth_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd } } -static int ionic_get_fw_vf_config(struct ionic *ionic, int vf, struct ionic_vf *vfdata) -{ - struct ionic_vf_getattr_comp comp = { 0 }; - int err; - u8 attr; - - attr = IONIC_VF_ATTR_VLAN; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) - vfdata->vlanid = comp.vlanid; - - attr = IONIC_VF_ATTR_SPOOFCHK; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) - vfdata->spoofchk = comp.spoofchk; - - attr = IONIC_VF_ATTR_LINKSTATE; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) { - switch (comp.linkstate) { - case IONIC_VF_LINK_STATUS_UP: - vfdata->linkstate = IFLA_VF_LINK_STATE_ENABLE; - break; - case IONIC_VF_LINK_STATUS_DOWN: - vfdata->linkstate = IFLA_VF_LINK_STATE_DISABLE; - break; - case IONIC_VF_LINK_STATUS_AUTO: - vfdata->linkstate = IFLA_VF_LINK_STATE_AUTO; - break; - default: - dev_warn(ionic->dev, "Unexpected link state %u\n", comp.linkstate); - break; - } - } - - attr = IONIC_VF_ATTR_RATE; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) - vfdata->maxrate = comp.maxrate; - - attr = IONIC_VF_ATTR_TRUST; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) - vfdata->trusted = comp.trust; - - attr = IONIC_VF_ATTR_MAC; - err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp); - if (err && comp.status != IONIC_RC_ENOSUPP) - goto err_out; - if (!err) - ether_addr_copy(vfdata->macaddr, comp.macaddr); - -err_out: - if (err) - dev_err(ionic->dev, "Failed to get %s for VF %d\n", - ionic_vf_attr_to_str(attr), vf); - - return err; -} - static int ionic_get_vf_config(struct net_device *netdev, int vf, struct ifla_vf_info *ivf) { struct ionic_lif *lif = netdev_priv(netdev); struct ionic *ionic = lif->ionic; - struct ionic_vf vfdata = { 0 }; int ret = 0; if (!netif_device_present(netdev)) @@ -2418,18 +2347,16 @@ static int ionic_get_vf_config(struct net_device *netdev, if (vf >= pci_num_vf(ionic->pdev) || !ionic->vfs) { ret = -EINVAL; } else { - ivf->vf = vf; - ivf->qos = 0; + struct ionic_vf *vfdata = &ionic->vfs[vf]; - ret = ionic_get_fw_vf_config(ionic, vf, &vfdata); - if (!ret) { - ivf->vlan = le16_to_cpu(vfdata.vlanid); - ivf->spoofchk = vfdata.spoofchk; - ivf->linkstate = vfdata.linkstate; - ivf->max_tx_rate = le32_to_cpu(vfdata.maxrate); - ivf->trusted = vfdata.trusted; - ether_addr_copy(ivf->mac, vfdata.macaddr); - } + ivf->vf = vf; + ivf->qos = 0; + ivf->vlan = le16_to_cpu(vfdata->vlanid); + ivf->spoofchk = vfdata->spoofchk; + ivf->linkstate = vfdata->linkstate; + ivf->max_tx_rate = le32_to_cpu(vfdata->maxrate); + ivf->trusted = vfdata->trusted; + ether_addr_copy(ivf->mac, vfdata->macaddr); } up_read(&ionic->vf_op_lock); diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c index 835577392178..8d15f9203bd5 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c @@ -188,28 +188,6 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode) } } -const char *ionic_vf_attr_to_str(enum ionic_vf_attr attr) -{ - switch (attr) { - case IONIC_VF_ATTR_SPOOFCHK: - return "IONIC_VF_ATTR_SPOOFCHK"; - case IONIC_VF_ATTR_TRUST: - return "IONIC_VF_ATTR_TRUST"; - case IONIC_VF_ATTR_LINKSTATE: - return "IONIC_VF_ATTR_LINKSTATE"; - case IONIC_VF_ATTR_MAC: - return "IONIC_VF_ATTR_MAC"; - case IONIC_VF_ATTR_VLAN: - return "IONIC_VF_ATTR_VLAN"; - case IONIC_VF_ATTR_RATE: - return "IONIC_VF_ATTR_RATE"; - case IONIC_VF_ATTR_STATSADDR: - return "IONIC_VF_ATTR_STATSADDR"; - default: - return "IONIC_VF_ATTR_UNKNOWN"; - } -} - static void ionic_adminq_flush(struct ionic_lif *lif) { struct ionic_desc_info *desc_info; From 46ca79d28fd7f2bbccf226fc0be59c2bd8d63dfe Mon Sep 17 00:00:00 2001 From: Shannon Nelson Date: Mon, 4 Dec 2023 13:09:33 -0800 Subject: [PATCH 2/5] ionic: set ionic ptr before setting up ethtool ops Set the lif->ionic value that is used in some ethtool callbacks before setting ethtool ops. There really shouldn't be any race issues before this change since the netdev hasn't been registered yet, but this seems more correct. Signed-off-by: Shannon Nelson Reviewed-by: Brett Creeley Reviewed-by: Florian Fainelli Reviewed-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20231204210936.16587-3-shannon.nelson@amd.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index afb77e2d04c5..a5e6b1e2f5ee 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -3054,6 +3054,7 @@ int ionic_lif_alloc(struct ionic *ionic) lif = netdev_priv(netdev); lif->netdev = netdev; ionic->lif = lif; + lif->ionic = ionic; netdev->netdev_ops = &ionic_netdev_ops; ionic_ethtool_set_ops(netdev); @@ -3076,7 +3077,6 @@ int ionic_lif_alloc(struct ionic *ionic) lif->neqs = ionic->neqs_per_lif; lif->nxqs = ionic->ntxqs_per_lif; - lif->ionic = ionic; lif->index = 0; if (is_kdump_kernel()) { From 2d0b80c3a550f7828f26dba029c2b9346be789af Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Mon, 4 Dec 2023 13:09:34 -0800 Subject: [PATCH 3/5] ionic: Don't check null when calling vfree() vfree() checks for null internally, so there's no need to check in the caller. So, always vfree() on variables allocated with valloc(). If the variables are never alloc'd vfree() is still safe. Signed-off-by: Brett Creeley Signed-off-by: Shannon Nelson Reviewed-by: Florian Fainelli Reviewed-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20231204210936.16587-4-shannon.nelson@amd.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/pensando/ionic/ionic_lif.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index a5e6b1e2f5ee..6842a31fc04b 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -424,14 +424,10 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq) ionic_qcq_intr_free(lif, qcq); - if (qcq->cq.info) { - vfree(qcq->cq.info); - qcq->cq.info = NULL; - } - if (qcq->q.info) { - vfree(qcq->q.info); - qcq->q.info = NULL; - } + vfree(qcq->cq.info); + qcq->cq.info = NULL; + vfree(qcq->q.info); + qcq->q.info = NULL; } void ionic_qcqs_free(struct ionic_lif *lif) From ab807e9183425ddf6dd85ca622a51fe4cad2c577 Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Mon, 4 Dec 2023 13:09:35 -0800 Subject: [PATCH 4/5] ionic: Make the check for Tx HW timestamping more obvious Currently the checks are: [1] unlikely(q->features & IONIC_TXQ_F_HWSTAMP) [2] !unlikely(q->features & IONIC_TXQ_F_HWSTAMP) [1] is clear enough, but [2] isn't exactly obvious to the reader because !unlikely reads as likely. However, that's not what this means. [2] means that it's unlikely that the q has IONIC_TXQ_F_HWSTAMP enabled. Write an inline helper function to hide the unlikely optimization to make these checks more readable. Signed-off-by: Brett Creeley Signed-off-by: Shannon Nelson Reviewed-by: Florian Fainelli Reviewed-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20231204210936.16587-5-shannon.nelson@amd.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/pensando/ionic/ionic_lif.h | 5 +++++ drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h index 457c24195ca6..61548b3eea93 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h @@ -312,6 +312,11 @@ static inline u32 ionic_coal_usec_to_hw(struct ionic *ionic, u32 usecs) return (usecs * mult) / div; } +static inline bool ionic_txq_hwstamp_enabled(struct ionic_queue *q) +{ + return unlikely(q->features & IONIC_TXQ_F_HWSTAMP); +} + void ionic_link_status_check_request(struct ionic_lif *lif, bool can_sleep); void ionic_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *ns); diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c index ccc1b1d407e4..54cd96b035d6 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c @@ -803,7 +803,7 @@ static void ionic_tx_clean(struct ionic_queue *q, qi = skb_get_queue_mapping(skb); - if (unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) { + if (ionic_txq_hwstamp_enabled(q)) { if (cq_info) { struct skb_shared_hwtstamps hwts = {}; __le64 *cq_desc_hwstamp; @@ -870,7 +870,7 @@ bool ionic_tx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info) desc_info->cb_arg = NULL; } while (index != le16_to_cpu(comp->comp_index)); - if (pkts && bytes && !unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) + if (pkts && bytes && !ionic_txq_hwstamp_enabled(q)) netdev_tx_completed_queue(q_to_ndq(q), pkts, bytes); return true; @@ -908,7 +908,7 @@ void ionic_tx_empty(struct ionic_queue *q) desc_info->cb_arg = NULL; } - if (pkts && bytes && !unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) + if (pkts && bytes && !ionic_txq_hwstamp_enabled(q)) netdev_tx_completed_queue(q_to_ndq(q), pkts, bytes); } @@ -986,7 +986,7 @@ static void ionic_tx_tso_post(struct ionic_queue *q, if (start) { skb_tx_timestamp(skb); - if (!unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) + if (!ionic_txq_hwstamp_enabled(q)) netdev_tx_sent_queue(q_to_ndq(q), skb->len); ionic_txq_post(q, false, ionic_tx_clean, skb); } else { @@ -1233,7 +1233,7 @@ static int ionic_tx(struct ionic_queue *q, struct sk_buff *skb) stats->pkts++; stats->bytes += skb->len; - if (!unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) + if (!ionic_txq_hwstamp_enabled(q)) netdev_tx_sent_queue(q_to_ndq(q), skb->len); ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb); From 5858036ca05658051ec61551e6699cca9c7d3369 Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Mon, 4 Dec 2023 13:09:36 -0800 Subject: [PATCH 5/5] ionic: Re-arrange ionic_intr_info struct for cache perf dim_coal_hw is accessed in the hotpath along with other values from the first cacheline of ionic_intr_info. So, re-arrange the structure so the hot path variables are on the first cacheline. Before: struct ionic_intr_info { char name[32]; /* 0 32 */ unsigned int index; /* 32 4 */ unsigned int vector; /* 36 4 */ u64 rearm_count; /* 40 8 */ unsigned int cpu; /* 48 4 */ /* XXX 4 bytes hole, try to pack */ cpumask_t affinity_mask; /* 56 1024 */ /* --- cacheline 16 boundary (1024 bytes) was 56 bytes ago --- */ u32 dim_coal_hw; /* 1080 4 */ /* size: 1088, cachelines: 17, members: 7 */ /* sum members: 1080, holes: 1, sum holes: 4 */ /* padding: 4 */ }; After: struct ionic_intr_info { char name[32]; /* 0 32 */ u64 rearm_count; /* 32 8 */ unsigned int index; /* 40 4 */ unsigned int vector; /* 44 4 */ unsigned int cpu; /* 48 4 */ u32 dim_coal_hw; /* 52 4 */ cpumask_t affinity_mask; /* 56 1024 */ /* size: 1080, cachelines: 17, members: 7 */ /* last cacheline: 56 bytes */ }; Signed-off-by: Brett Creeley Signed-off-by: Shannon Nelson Reviewed-by: Florian Fainelli Reviewed-by: Rahul Rameshbabu Link: https://lore.kernel.org/r/20231204210936.16587-6-shannon.nelson@amd.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/pensando/ionic/ionic_dev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h index 19edcb42d9fd..cee4e5c3d09a 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h @@ -269,12 +269,12 @@ struct ionic_queue { struct ionic_intr_info { char name[IONIC_INTR_NAME_MAX_SZ]; + u64 rearm_count; unsigned int index; unsigned int vector; - u64 rearm_count; unsigned int cpu; - cpumask_t affinity_mask; u32 dim_coal_hw; + cpumask_t affinity_mask; }; struct ionic_cq {