diff mbox series

[net-next,2/3] octeontx2-pf: Add devlink param to vary cqe size

Message ID 1633454136-14679-3-git-send-email-sbhatta@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add devlink params to vary cqe and rbuf | expand

Checks

Context Check Description
netdev/cover_letter success Series has a cover letter
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns
netdev/build_allmodconfig_warn fail Errors and warnings before: 23 this patch: 23
netdev/header_inline success No static functions without inline keyword in header files

Commit Message

Subbaraya Sundeep Oct. 5, 2021, 5:15 p.m. UTC 
Completion Queue Entry(CQE) is a descriptor written
by hardware to notify software about the send and
receive completion status. The CQE can be of size
128 or 512 bytes. A 512 bytes CQE can hold more receive
fragments pointers compared to 128 bytes CQE. This
patch adds devlink param to change CQE descriptor
size.

Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 .../ethernet/marvell/octeontx2/nic/otx2_common.c   | 10 +++-
 .../ethernet/marvell/octeontx2/nic/otx2_common.h   |  1 +
 .../ethernet/marvell/octeontx2/nic/otx2_devlink.c  | 56 ++++++++++++++++++++++
 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c   |  2 +
 .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c   |  2 +
 5 files changed, 69 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Oct. 6, 2021, 1:11 a.m. UTC | #1 
On Tue, 5 Oct 2021 22:45:35 +0530 Subbaraya Sundeep wrote:
> Completion Queue Entry(CQE) is a descriptor written
> by hardware to notify software about the send and
> receive completion status. The CQE can be of size
> 128 or 512 bytes. A 512 bytes CQE can hold more receive
> fragments pointers compared to 128 bytes CQE. This
> patch adds devlink param to change CQE descriptor
> size.

nak, this belongs in ethtool -g
sundeep subbaraya Oct. 6, 2021, 6:59 a.m. UTC | #2 
Hi Jakub,

On Wed, Oct 6, 2021 at 6:46 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 5 Oct 2021 22:45:35 +0530 Subbaraya Sundeep wrote:
> > Completion Queue Entry(CQE) is a descriptor written
> > by hardware to notify software about the send and
> > receive completion status. The CQE can be of size
> > 128 or 512 bytes. A 512 bytes CQE can hold more receive
> > fragments pointers compared to 128 bytes CQE. This
> > patch adds devlink param to change CQE descriptor
> > size.
>
> nak, this belongs in ethtool -g

We do use ethtool -G for setting the number of receive buffers to
allocate from the kernel and give those pointers to hardware memory pool(NPA).

This patch is to specify hardware the completion queue descriptor size
it needs to use
while writing to memory. The CQE consists of buffer pointer/packet addresses.
Say a large packet is received then hardware splits that large packet
into buffers
and writes only one CQE consisting of all the buffer pointers which
makes a packet.
If hardware is configured to use 128 byte CQE then only 6 pointers can
be accomodated
and rest of packet data is truncated. If CQE is configured as 512 byte
then 42 pointers
can be accomodated hence large packets can be received.
ethtool -G can be used to change number of packets a ring can hold but
not number
of fragments a single packet can use. Since this is hardware related
am using devlink.
CN9XX series hardware max packet length is 9212 and a 128 byte CQE (6
buffer pointers)
with 2K receive buffer was sufficient to receive 9212 packet (2k * 6 =
12K). CN10XX series
max receive length is 65535 so 128 byte CQE was not enough.



Thanks,
Sundeep
Jakub Kicinski Oct. 6, 2021, 1:40 p.m. UTC | #3 
On Wed, 6 Oct 2021 12:29:51 +0530 sundeep subbaraya wrote:
> We do use ethtool -G for setting the number of receive buffers to
> allocate from the kernel and give those pointers to hardware memory pool(NPA).

You can extend the ethtool API.
sundeep subbaraya Oct. 8, 2021, 7:12 a.m. UTC | #4 
Hi Jakub,

On Wed, Oct 6, 2021 at 7:10 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 6 Oct 2021 12:29:51 +0530 sundeep subbaraya wrote:
> > We do use ethtool -G for setting the number of receive buffers to
> > allocate from the kernel and give those pointers to hardware memory pool(NPA).
>
> You can extend the ethtool API.

I will rework on this patch. Is it okay I drop this patch in this
series and send only patches 1 and 3 for v3?

Thanks,
Sundeep
Jakub Kicinski Oct. 8, 2021, 2:11 p.m. UTC | #5 
On Fri, 8 Oct 2021 12:42:34 +0530 sundeep subbaraya wrote:
> On Wed, Oct 6, 2021 at 7:10 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 6 Oct 2021 12:29:51 +0530 sundeep subbaraya wrote:  
> > > We do use ethtool -G for setting the number of receive buffers to
> > > allocate from the kernel and give those pointers to hardware memory pool(NPA).  
> >
> > You can extend the ethtool API.  
> 
> I will rework on this patch. Is it okay I drop this patch in this
> series and send only patches 1 and 3 for v3?

The first patch looks fine. But the last is where I think a common
interface is most likely to succeed, so no, patch 3 is not fine. 

The documentation (which BTW is required for devlink params) is lacking
so I can't be sure but patch 3 looks similar to what Huawei has been
working on, take a look:

https://lore.kernel.org/all/20210924142959.7798-4-huangguangbin2@huawei.com/
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 66da31f..3777f41 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -222,8 +222,11 @@  EXPORT_SYMBOL(otx2_set_mac_address);
 int otx2_hw_set_mtu(struct otx2_nic *pfvf, int mtu)
 {
 	struct nix_frs_cfg *req;
+	u16 maxlen;
 	int err;
 
+	maxlen = otx2_get_max_mtu(pfvf) + OTX2_ETH_HLEN + OTX2_HW_TIMESTAMP_LEN;
+
 	mutex_lock(&pfvf->mbox.lock);
 	req = otx2_mbox_alloc_msg_nix_set_hw_frs(&pfvf->mbox);
 	if (!req) {
@@ -233,6 +236,9 @@  int otx2_hw_set_mtu(struct otx2_nic *pfvf, int mtu)
 
 	req->maxlen = pfvf->netdev->mtu + OTX2_ETH_HLEN + OTX2_HW_TIMESTAMP_LEN;
 
+	if (is_otx2_lbkvf(pfvf->pdev))
+		req->maxlen = maxlen;
+
 	err = otx2_sync_mbox_msg(&pfvf->mbox);
 	mutex_unlock(&pfvf->mbox.lock);
 	return err;
@@ -1036,7 +1042,7 @@  int otx2_config_nix(struct otx2_nic *pfvf)
 	struct nix_lf_alloc_rsp *rsp;
 	int err;
 
-	pfvf->qset.xqe_size = NIX_XQESZ_W16 ? 128 : 512;
+	pfvf->qset.xqe_size = pfvf->hw.xqe_size;
 
 	/* Get memory to put this msg */
 	nixlf = otx2_mbox_alloc_msg_nix_lf_alloc(&pfvf->mbox);
@@ -1049,7 +1055,7 @@  int otx2_config_nix(struct otx2_nic *pfvf)
 	nixlf->cq_cnt = pfvf->qset.cq_cnt;
 	nixlf->rss_sz = MAX_RSS_INDIR_TBL_SIZE;
 	nixlf->rss_grps = MAX_RSS_GROUPS;
-	nixlf->xqe_sz = NIX_XQESZ_W16;
+	nixlf->xqe_sz = pfvf->hw.xqe_size == 128 ? NIX_XQESZ_W16 : NIX_XQESZ_W64;
 	/* We don't know absolute NPA LF idx attached.
 	 * AF will replace 'RVU_DEFAULT_PF_FUNC' with
 	 * NPA LF attached to this RVU PF/VF.
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 61e5281..6e0d1ac 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -177,6 +177,7 @@  struct otx2_hw {
 	u16			pool_cnt;
 	u16			rqpool_cnt;
 	u16			sqpool_cnt;
+	u16			xqe_size;
 
 	/* NPA */
 	u32			stack_pg_ptrs;  /* No of ptrs per stack page */
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 777a270..98450e1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -64,9 +64,60 @@  static int otx2_dl_mcam_count_get(struct devlink *devlink, u32 id,
 	return 0;
 }
 
+static int otx2_dl_cqe_size_validate(struct devlink *devlink, u32 id,
+				     union devlink_param_value val,
+				     struct netlink_ext_ack *extack)
+{
+	if (val.vu16 != 128 && val.vu16 != 512) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Only 128 or 512 byte descriptor allowed");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int otx2_dl_cqe_size_set(struct devlink *devlink, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
+	struct otx2_nic *pfvf = otx2_dl->pfvf;
+	struct net_device *netdev;
+	int err = 0;
+	bool if_up;
+
+	rtnl_lock();
+
+	netdev = pfvf->netdev;
+	if_up = netif_running(netdev);
+	if (if_up)
+		netdev->netdev_ops->ndo_stop(netdev);
+
+	pfvf->hw.xqe_size = ctx->val.vu16;
+
+	if (if_up)
+		err = netdev->netdev_ops->ndo_open(netdev);
+
+	rtnl_unlock();
+
+	return err;
+}
+
+static int otx2_dl_cqe_size_get(struct devlink *devlink, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
+	struct otx2_nic *pfvf = otx2_dl->pfvf;
+
+	ctx->val.vu16 = pfvf->hw.xqe_size;
+
+	return 0;
+}
+
 enum otx2_dl_param_id {
 	OTX2_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
 	OTX2_DEVLINK_PARAM_ID_MCAM_COUNT,
+	OTX2_DEVLINK_PARAM_ID_CQE_SIZE,
 };
 
 static const struct devlink_param otx2_dl_params[] = {
@@ -75,6 +126,11 @@  static const struct devlink_param otx2_dl_params[] = {
 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
 			     otx2_dl_mcam_count_get, otx2_dl_mcam_count_set,
 			     otx2_dl_mcam_count_validate),
+	DEVLINK_PARAM_DRIVER(OTX2_DEVLINK_PARAM_ID_CQE_SIZE,
+			     "completion_descriptor_size", DEVLINK_PARAM_TYPE_U16,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     otx2_dl_cqe_size_get, otx2_dl_cqe_size_set,
+			     otx2_dl_cqe_size_validate),
 };
 
 /* Devlink OPs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 1e0d0c9c..8618cf7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2624,6 +2624,8 @@  static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	hw->tx_queues = qcount;
 	hw->tot_tx_queues = qcount;
 	hw->max_queues = qcount;
+	/* Use CQE of 128 byte descriptor size by default */
+	hw->xqe_size = 128;
 
 	num_vec = pci_msix_vec_count(pdev);
 	hw->irq_name = devm_kmalloc_array(&hw->pdev->dev, num_vec, NAME_SIZE,
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index 980219a..672be05 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -587,6 +587,8 @@  static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	hw->tx_queues = qcount;
 	hw->max_queues = qcount;
 	hw->tot_tx_queues = qcount;
+	/* Use CQE of 128 byte descriptor size by default */
+	hw->xqe_size = 128;
 
 	hw->irq_name = devm_kmalloc_array(&hw->pdev->dev, num_vec, NAME_SIZE,
 					  GFP_KERNEL);