[v2,14/18] net/dpaa: add Tx rate limiting DPAA PMD API

Message ID 20240823073240.3708320-15-hemant.agrawal@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Ferruh Yigit
Headers
Series NXP DPAA ETH driver enhancement and fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hemant Agrawal Aug. 23, 2024, 7:32 a.m. UTC
From: Vinod Pullabhatla <vinod.pullabhatla@nxp.com>

Add support to set Tx rate on DPAA platform through PMD APIs

Signed-off-by: Vinod Pullabhatla <vinod.pullabhatla@nxp.com>
Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
---
 .mailmap                             |  1 +
 drivers/net/dpaa/dpaa_flow.c         | 95 +++++++++++++++++++++++++---
 drivers/net/dpaa/fmlib/fm_lib.c      | 32 +++++++++-
 drivers/net/dpaa/fmlib/fm_port_ext.h |  2 +-
 drivers/net/dpaa/rte_pmd_dpaa.h      | 25 +++++++-
 drivers/net/dpaa/version.map         |  7 ++
 6 files changed, 151 insertions(+), 11 deletions(-)
  

Comments

Ferruh Yigit Sept. 22, 2024, 3:14 a.m. UTC | #1
On 8/23/2024 8:32 AM, Hemant Agrawal wrote:
> diff --git a/drivers/net/dpaa/version.map b/drivers/net/dpaa/version.map
> index 3fdb63caf3..24a28ce649 100644
> --- a/drivers/net/dpaa/version.map
> +++ b/drivers/net/dpaa/version.map
> @@ -6,6 +6,13 @@ DPDK_25 {
>  	local: *;
>  };
>  
> +EXPERIMENTAL {
> +	global:
> +
> +	# added in 24.11
> +	rte_pmd_dpaa_port_set_rate_limit;
> +};
> +
>  INTERNAL {
>  	global:
>

PMD specific API needs to be justified, can't we use TM framework for
this, does TM needs to be improved for this support?

What do you think to send the rest of the set without this patch, so
they can progress, and this one can be discussed separately (assuming
there is no dependency).
  
Hemant Agrawal Sept. 22, 2024, 4:40 a.m. UTC | #2
HI Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Sunday, September 22, 2024 8:44 AM
> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
> Cc: ferruh.yigit@intel.com; Vinod Pullabhatla <vinod.pullabhatla@nxp.com>;
> Rohit Raj <rohit.raj@nxp.com>
> Subject: Re: [PATCH v2 14/18] net/dpaa: add Tx rate limiting DPAA PMD API
> Importance: High
> 
> On 8/23/2024 8:32 AM, Hemant Agrawal wrote:
> > diff --git a/drivers/net/dpaa/version.map
> > b/drivers/net/dpaa/version.map index 3fdb63caf3..24a28ce649 100644
> > --- a/drivers/net/dpaa/version.map
> > +++ b/drivers/net/dpaa/version.map
> > @@ -6,6 +6,13 @@ DPDK_25 {
> >  	local: *;
> >  };
> >
> > +EXPERIMENTAL {
> > +	global:
> > +
> > +	# added in 24.11
> > +	rte_pmd_dpaa_port_set_rate_limit;
> > +};
> > +
> >  INTERNAL {
> >  	global:
> >
> 
> PMD specific API needs to be justified, can't we use TM framework for this,
> does TM needs to be improved for this support?
> 
> What do you think to send the rest of the set without this patch, so they can
> progress, and this one can be discussed separately (assuming there is no
> dependency).

[Hemant] I think, I replied to your earlier concerns. 

We are yet to implement TM framework for DPAA1.  But that involves more of egress QoS.

This one is additional capability to limit the ingress port. Kind of policing in Rx side.

However, if you still disagree. Please apply the series without this patch.
  
Ferruh Yigit Sept. 22, 2024, 1:10 p.m. UTC | #3
On 9/22/2024 5:40 AM, Hemant Agrawal wrote:
> HI Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Sunday, September 22, 2024 8:44 AM
>> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
>> Cc: ferruh.yigit@intel.com; Vinod Pullabhatla <vinod.pullabhatla@nxp.com>;
>> Rohit Raj <rohit.raj@nxp.com>
>> Subject: Re: [PATCH v2 14/18] net/dpaa: add Tx rate limiting DPAA PMD API
>> Importance: High
>>
>> On 8/23/2024 8:32 AM, Hemant Agrawal wrote:
>>> diff --git a/drivers/net/dpaa/version.map
>>> b/drivers/net/dpaa/version.map index 3fdb63caf3..24a28ce649 100644
>>> --- a/drivers/net/dpaa/version.map
>>> +++ b/drivers/net/dpaa/version.map
>>> @@ -6,6 +6,13 @@ DPDK_25 {
>>>  	local: *;
>>>  };
>>>
>>> +EXPERIMENTAL {
>>> +	global:
>>> +
>>> +	# added in 24.11
>>> +	rte_pmd_dpaa_port_set_rate_limit;
>>> +};
>>> +
>>>  INTERNAL {
>>>  	global:
>>>
>>
>> PMD specific API needs to be justified, can't we use TM framework for this,
>> does TM needs to be improved for this support?
>>
>> What do you think to send the rest of the set without this patch, so they can
>> progress, and this one can be discussed separately (assuming there is no
>> dependency).
> 
> [Hemant] I think, I replied to your earlier concerns. 
> 
> We are yet to implement TM framework for DPAA1.  But that involves more of egress QoS.
> 
> This one is additional capability to limit the ingress port. Kind of policing in Rx side.
> 
> However, if you still disagree. Please apply the series without this patch.
>

Let's detect what is missing in ethdev layer for it, and what is the
required effort to cover it in ethdev, first. Based on findings, we can
continue with PMD API as last resort.

I will continue with patch series without this path.
  
Ferruh Yigit Sept. 22, 2024, 1:27 p.m. UTC | #4
On 9/22/2024 5:40 AM, Hemant Agrawal wrote:
> HI Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Sunday, September 22, 2024 8:44 AM
>> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
>> Cc: ferruh.yigit@intel.com; Vinod Pullabhatla <vinod.pullabhatla@nxp.com>;
>> Rohit Raj <rohit.raj@nxp.com>
>> Subject: Re: [PATCH v2 14/18] net/dpaa: add Tx rate limiting DPAA PMD API
>> Importance: High
>>
>> On 8/23/2024 8:32 AM, Hemant Agrawal wrote:
>>> diff --git a/drivers/net/dpaa/version.map
>>> b/drivers/net/dpaa/version.map index 3fdb63caf3..24a28ce649 100644
>>> --- a/drivers/net/dpaa/version.map
>>> +++ b/drivers/net/dpaa/version.map
>>> @@ -6,6 +6,13 @@ DPDK_25 {
>>>  	local: *;
>>>  };
>>>
>>> +EXPERIMENTAL {
>>> +	global:
>>> +
>>> +	# added in 24.11
>>> +	rte_pmd_dpaa_port_set_rate_limit;
>>> +};
>>> +
>>>  INTERNAL {
>>>  	global:
>>>
>>
>> PMD specific API needs to be justified, can't we use TM framework for this,
>> does TM needs to be improved for this support?
>>
>> What do you think to send the rest of the set without this patch, so they can
>> progress, and this one can be discussed separately (assuming there is no
>> dependency).
> 
> [Hemant] I think, I replied to your earlier concerns. 
> 
> We are yet to implement TM framework for DPAA1.  But that involves more of egress QoS.
> 
> This one is additional capability to limit the ingress port. Kind of policing in Rx side.
> 
> However, if you still disagree. Please apply the series without this patch.
>

Let's detect what is missing in ethdev layer for it, and what is the
required effort to cover it in ethdev, first. Based on findings, we can
continue with PMD API as last resort.

I will continue with patch series without this path.
  
Ferruh Yigit Sept. 22, 2024, 3:23 p.m. UTC | #5
On 9/22/2024 2:27 PM, Ferruh Yigit wrote:
> On 9/22/2024 5:40 AM, Hemant Agrawal wrote:
>> HI Ferruh,
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>> Sent: Sunday, September 22, 2024 8:44 AM
>>> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
>>> Cc: ferruh.yigit@intel.com; Vinod Pullabhatla <vinod.pullabhatla@nxp.com>;
>>> Rohit Raj <rohit.raj@nxp.com>
>>> Subject: Re: [PATCH v2 14/18] net/dpaa: add Tx rate limiting DPAA PMD API
>>> Importance: High
>>>
>>> On 8/23/2024 8:32 AM, Hemant Agrawal wrote:
>>>> diff --git a/drivers/net/dpaa/version.map
>>>> b/drivers/net/dpaa/version.map index 3fdb63caf3..24a28ce649 100644
>>>> --- a/drivers/net/dpaa/version.map
>>>> +++ b/drivers/net/dpaa/version.map
>>>> @@ -6,6 +6,13 @@ DPDK_25 {
>>>>  	local: *;
>>>>  };
>>>>
>>>> +EXPERIMENTAL {
>>>> +	global:
>>>> +
>>>> +	# added in 24.11
>>>> +	rte_pmd_dpaa_port_set_rate_limit;
>>>> +};
>>>> +
>>>>  INTERNAL {
>>>>  	global:
>>>>
>>>
>>> PMD specific API needs to be justified, can't we use TM framework for this,
>>> does TM needs to be improved for this support?
>>>
>>> What do you think to send the rest of the set without this patch, so they can
>>> progress, and this one can be discussed separately (assuming there is no
>>> dependency).
>>
>> [Hemant] I think, I replied to your earlier concerns. 
>>
>> We are yet to implement TM framework for DPAA1.  But that involves more of egress QoS.
>>
>> This one is additional capability to limit the ingress port. Kind of policing in Rx side.
>>
>> However, if you still disagree. Please apply the series without this patch.
>>
> 
> Let's detect what is missing in ethdev layer for it, and what is the
> required effort to cover it in ethdev, first. Based on findings, we can
> continue with PMD API as last resort.
> 
> I will continue with patch series without this path.
>

Hi Hemant,

I removed the commit 14/18 from set, but it impacts other patches, I can
fix the build but can't verify the functionality, probably better if you
send a new version without patch 14/18.

Btw, while resolving conflict I recognized that patch 15/18 renames
'fman_offline_internal' -> 'fman_offline', and next patch (16/18),
renames it back to 'fman_offline_internal', this creates lots of noise
in both patches, can this be prevented, I will comment on the patch?
  

Patch

diff --git a/.mailmap b/.mailmap
index 4a508bafad..cb0fd52404 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1562,6 +1562,7 @@  Vincent Jardin <vincent.jardin@6wind.com>
 Vincent Li <vincent.mc.li@gmail.com>
 Vincent S. Cojot <vcojot@redhat.com>
 Vinh Tran <vinh.t.tran10@gmail.com>
+Vinod Pullabhatla <vinod.pullabhatla@nxp.com>
 Vipin Padmam Ramesh <vipinp@vmware.com>
 Vipin Varghese <vipin.varghese@amd.com> <vipin.varghese@intel.com>
 Vipul Ashri <vipul.ashri@oracle.com>
diff --git a/drivers/net/dpaa/dpaa_flow.c b/drivers/net/dpaa/dpaa_flow.c
index 082bd5d014..dfc81e4e43 100644
--- a/drivers/net/dpaa/dpaa_flow.c
+++ b/drivers/net/dpaa/dpaa_flow.c
@@ -13,6 +13,7 @@ 
 #include <rte_dpaa_logs.h>
 #include <fmlib/fm_port_ext.h>
 #include <fmlib/fm_vsp_ext.h>
+#include <rte_pmd_dpaa.h>
 
 #define DPAA_MAX_NUM_ETH_DEV	8
 
@@ -29,6 +30,11 @@  return &scheme_params->param.key_ext_and_hash.extract_array[hdr_idx];
 #define SCH_EXT_FULL_FLD(scheme_params, hdr_idx) \
 	SCH_EXT_HDR(scheme_params, hdr_idx).extract_by_hdr_type.full_field
 
+/* FMAN mac indexes mappings (0 is unused, first 8 are for 1G, next for 10G
+ * ports).
+ */
+const uint8_t mac_idx[] = {-1, 0, 1, 2, 3, 4, 5, 6, 7, 0, 1};
+
 /* FM global info */
 struct dpaa_fm_info {
 	t_handle fman_handle;
@@ -649,7 +655,7 @@  static inline int set_pcd_netenv_scheme(struct dpaa_if *dpaa_intf,
 }
 
 
-static inline int get_port_type(struct fman_if *fif)
+static inline int get_rx_port_type(struct fman_if *fif)
 {
 	/* For 1G fm-mac9 and fm-mac10 ports, configure the VSP as 10G
 	 * ports so that kernel can configure correct port.
@@ -668,6 +674,19 @@  static inline int get_port_type(struct fman_if *fif)
 	return -1;
 }
 
+static inline int get_tx_port_type(struct fman_if *fif)
+{
+	if (fif->mac_type == fman_mac_1g)
+		return e_FM_PORT_TYPE_TX;
+	else if (fif->mac_type == fman_mac_2_5g)
+		return e_FM_PORT_TYPE_TX_2_5G;
+	else if (fif->mac_type == fman_mac_10g)
+		return e_FM_PORT_TYPE_TX_10G;
+
+	DPAA_PMD_ERR("MAC type unsupported");
+	return -1;
+}
+
 static inline int set_fm_port_handle(struct dpaa_if *dpaa_intf,
 				     uint64_t req_dist_set,
 				     struct fman_if *fif)
@@ -676,17 +695,12 @@  static inline int set_fm_port_handle(struct dpaa_if *dpaa_intf,
 	ioc_fm_pcd_net_env_params_t dist_units;
 	PMD_INIT_FUNC_TRACE();
 
-	/* FMAN mac indexes mappings (0 is unused,
-	 * first 8 are for 1G, next for 10G ports
-	 */
-	uint8_t mac_idx[] = {-1, 0, 1, 2, 3, 4, 5, 6, 7, 0, 1};
-
 	/* Memset FM port params */
 	memset(&fm_port_params, 0, sizeof(fm_port_params));
 
 	/* Set FM port params */
 	fm_port_params.h_fm = fm_info.fman_handle;
-	fm_port_params.port_type = get_port_type(fif);
+	fm_port_params.port_type = get_rx_port_type(fif);
 	fm_port_params.port_id = mac_idx[fif->mac_idx];
 
 	/* FM PORT Open */
@@ -949,7 +963,6 @@  static int dpaa_port_vsp_configure(struct dpaa_if *dpaa_intf,
 {
 	t_fm_vsp_params vsp_params;
 	t_fm_buffer_prefix_content buf_prefix_cont;
-	uint8_t mac_idx[] = {-1, 0, 1, 2, 3, 4, 5, 6, 7, 0, 1};
 	uint8_t idx = mac_idx[fif->mac_idx];
 	int ret;
 
@@ -1079,3 +1092,69 @@  int dpaa_port_vsp_cleanup(struct dpaa_if *dpaa_intf, struct fman_if *fif)
 
 	return E_OK;
 }
+
+int rte_pmd_dpaa_port_set_rate_limit(uint16_t port_id, uint16_t burst,
+				     uint32_t rate)
+{
+	t_fm_port_rate_limit port_rate_limit;
+	bool port_handle_exists = true;
+	void *handle;
+	uint32_t ret;
+	struct rte_eth_dev *dev;
+	struct dpaa_if *dpaa_intf;
+	struct fman_if *fif;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+	dpaa_intf = dev->data->dev_private;
+	fif = dev->process_private;
+
+	memset(&port_rate_limit, 0, sizeof(port_rate_limit));
+	port_rate_limit.max_burst_size = burst;
+	port_rate_limit.rate_limit = rate;
+
+	DPAA_PMD_DEBUG("Setting Rate Limiter for port:%s  Max Burst =%u Max Rate =%u\n",
+		       dpaa_intf->name, burst, rate);
+
+	if (!dpaa_intf->port_handle) {
+		t_fm_port_params fm_port_params;
+
+		/* Memset FM port params */
+		memset(&fm_port_params, 0, sizeof(fm_port_params));
+
+		/* Set FM port params */
+		fm_port_params.h_fm = fm_open(0);
+		fm_port_params.port_type = get_tx_port_type(fif);
+		fm_port_params.port_id = mac_idx[fif->mac_idx];
+
+		/* FM PORT Open */
+		handle = fm_port_open(&fm_port_params);
+		fm_close(fm_port_params.h_fm);
+		if (!handle) {
+			DPAA_PMD_ERR("Can't open handle %p\n",
+				     fm_info.fman_handle);
+			return -ENODEV;
+		}
+
+		port_handle_exists = false;
+	} else {
+		handle = dpaa_intf->port_handle;
+	}
+
+	if (burst == 0 || rate == 0)
+		ret = fm_port_delete_rate_limit(handle);
+	else
+		ret = fm_port_set_rate_limit(handle, &port_rate_limit);
+
+	if (ret) {
+		DPAA_PMD_ERR("Failed to set rate limit ret = %#x\n", -ret);
+		return -ret;
+	}
+
+	DPAA_PMD_DEBUG("FM_PORT_SetRateLimit ret = %#x\n", -ret);
+
+	if (!port_handle_exists)
+		fm_port_close(handle);
+
+	return 0;
+}
diff --git a/drivers/net/dpaa/fmlib/fm_lib.c b/drivers/net/dpaa/fmlib/fm_lib.c
index 68b519ff8a..4b7dd38496 100644
--- a/drivers/net/dpaa/fmlib/fm_lib.c
+++ b/drivers/net/dpaa/fmlib/fm_lib.c
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright 2008-2016 Freescale Semiconductor Inc.
- * Copyright 2017-2020 NXP
+ * Copyright 2017-2020,2022 NXP
  */
 
 #include <stdio.h>
@@ -558,3 +558,33 @@  get_device_id(t_handle h_dev)
 
 	return (t_handle)p_dev->id;
 }
+
+uint32_t
+fm_port_delete_rate_limit(t_handle h_fm_port)
+{
+	t_device        *p_dev = (t_device *)h_fm_port;
+
+	_fml_dbg("Calling...\n");
+
+	if (ioctl(p_dev->fd, FM_PORT_IOC_REMOVE_RATE_LIMIT))
+		RETURN_ERROR(MINOR, E_INVALID_OPERATION, NO_MSG);
+
+	_fml_dbg("Finishing.\n");
+
+	return E_OK;
+}
+
+uint32_t
+fm_port_set_rate_limit(t_handle h_fm_port, t_fm_port_rate_limit *p_rate_limit)
+{
+	t_device        *p_dev = (t_device *)h_fm_port;
+
+	_fml_dbg("Calling...\n");
+
+	if (ioctl(p_dev->fd, FM_PORT_IOC_SET_RATE_LIMIT, p_rate_limit))
+		RETURN_ERROR(MINOR, E_INVALID_OPERATION, NO_MSG);
+
+	_fml_dbg("Finishing.\n");
+
+	return E_OK;
+}
diff --git a/drivers/net/dpaa/fmlib/fm_port_ext.h b/drivers/net/dpaa/fmlib/fm_port_ext.h
index bb2e00222e..f1cbf37de3 100644
--- a/drivers/net/dpaa/fmlib/fm_port_ext.h
+++ b/drivers/net/dpaa/fmlib/fm_port_ext.h
@@ -274,7 +274,7 @@  typedef struct ioc_fm_port_congestion_groups_t {
  * @Return	0 on success; error code otherwise.
  */
 #define FM_PORT_IOC_SET_RATE_LIMIT \
-	IOW(FM_IOC_TYPE_BASE, FM_PORT_IOC_NUM(3), ioc_fm_port_rate_limit_t)
+	_IOW(FM_IOC_TYPE_BASE, FM_PORT_IOC_NUM(3), ioc_fm_port_rate_limit_t)
 
 /*
  * @Function	  fm_port_delete_rate_limit
diff --git a/drivers/net/dpaa/rte_pmd_dpaa.h b/drivers/net/dpaa/rte_pmd_dpaa.h
index ec45633ba2..b48adff570 100644
--- a/drivers/net/dpaa/rte_pmd_dpaa.h
+++ b/drivers/net/dpaa/rte_pmd_dpaa.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2018 NXP
+ * Copyright 2018,2022 NXP
  */
 
 #ifndef _PMD_DPAA_H_
@@ -31,4 +31,27 @@ 
 int
 rte_pmd_dpaa_set_tx_loopback(uint16_t port, uint8_t on);
 
+/**
+ * Set TX rate limit
+ *
+ * @param port_id
+ *    The port identifier of the Ethernet device.
+ * @param burst
+ *    Max burst size(KBytes) of the Ethernet device.
+ *    0 - Disable TX rate limit.
+ * @param rate
+ *    Max rate(Kb/sec) of the Ethernet device.
+ *    0 - Disable TX rate limit.
+ * @return
+ *    0 - if successful.
+ *    <0 - if failed, with proper error code.
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ */
+__rte_experimental
+int
+rte_pmd_dpaa_port_set_rate_limit(uint16_t port_id, uint16_t burst,
+				 uint32_t rate);
+
 #endif /* _PMD_DPAA_H_ */
diff --git a/drivers/net/dpaa/version.map b/drivers/net/dpaa/version.map
index 3fdb63caf3..24a28ce649 100644
--- a/drivers/net/dpaa/version.map
+++ b/drivers/net/dpaa/version.map
@@ -6,6 +6,13 @@  DPDK_25 {
 	local: *;
 };
 
+EXPERIMENTAL {
+	global:
+
+	# added in 24.11
+	rte_pmd_dpaa_port_set_rate_limit;
+};
+
 INTERNAL {
 	global: