[2/2] net/nfp: enhance the flower service framework

Message ID 20241010074557.3622716-3-chaoyong.he@corigine.com (mailing list archive)
State Accepted
Delegated to: Ferruh Yigit
Headers
Series enhance the flower service framework |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build fail github build: failed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing fail Testing issues
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing warning Testing issues
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Chaoyong He Oct. 10, 2024, 7:45 a.m. UTC
From: Long Wu <long.wu@corigine.com>

Some DPDK applications may not have the service core which
can be used for the NFP flower service, so enhance the flower
service framework by adding an alarm for this situation.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower_service.c | 70 +++++++++++++++++++--
 1 file changed, 65 insertions(+), 5 deletions(-)
  

Comments

Stephen Hemminger Oct. 10, 2024, 2:55 p.m. UTC | #1
On Thu, 10 Oct 2024 15:45:57 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> +static void
> +nfp_flower_service_alarm_func(void *arg)
> +{
> +	int ret;
> +	uint16_t slot;
> +	struct nfp_net_hw_priv *hw_priv;
> +	struct nfp_flower_service *service_handle;
> +
> +	service_handle = arg;
> +	if (!service_handle->alarm_enabled)
> +		goto alarm_set;
> +
> +	rte_spinlock_lock(&service_handle->spinlock);
> +	for (slot = 0; slot < MAX_FLOWER_SERVICE_SLOT; slot++) {
> +		hw_priv = service_handle->slots[slot];
> +		if (hw_priv == NULL)
> +			continue;
> +
> +		nfp_flower_ctrl_vnic_process(hw_priv);
> +	}
> +	rte_spinlock_unlock(&service_handle->spinlock);
> +

The alarm handling is in a non EAL thread, and service is an lcore;
does that matter to this code?
  
Chaoyong He Oct. 11, 2024, 2:23 a.m. UTC | #2
> On Thu, 10 Oct 2024 15:45:57 +0800
> Chaoyong He <chaoyong.he@corigine.com> wrote:
> 
> > +static void
> > +nfp_flower_service_alarm_func(void *arg) {
> > +	int ret;
> > +	uint16_t slot;
> > +	struct nfp_net_hw_priv *hw_priv;
> > +	struct nfp_flower_service *service_handle;
> > +
> > +	service_handle = arg;
> > +	if (!service_handle->alarm_enabled)
> > +		goto alarm_set;
> > +
> > +	rte_spinlock_lock(&service_handle->spinlock);
> > +	for (slot = 0; slot < MAX_FLOWER_SERVICE_SLOT; slot++) {
> > +		hw_priv = service_handle->slots[slot];
> > +		if (hw_priv == NULL)
> > +			continue;
> > +
> > +		nfp_flower_ctrl_vnic_process(hw_priv);
> > +	}
> > +	rte_spinlock_unlock(&service_handle->spinlock);
> > +
> 
> The alarm handling is in a non EAL thread, and service is an lcore; does that
> matter to this code?

It does not matter to this code, maybe the name of this function and some variables introduced a few doubts, but the logic is okay.
Thanks for the review.
  

Patch

diff --git a/drivers/net/nfp/flower/nfp_flower_service.c b/drivers/net/nfp/flower/nfp_flower_service.c
index 782f083b71..aac11dbb94 100644
--- a/drivers/net/nfp/flower/nfp_flower_service.c
+++ b/drivers/net/nfp/flower/nfp_flower_service.c
@@ -5,6 +5,7 @@ 
 
 #include "nfp_flower_service.h"
 
+#include <rte_alarm.h>
 #include <rte_spinlock.h>
 
 #include "nfp_flower_ctrl.h"
@@ -16,9 +17,13 @@ 
 /* Driver limitation, PMD can enlarge it if need. */
 #define MAX_FLOWER_SERVICE_SLOT 8
 
+#define FLOWER_ALARM_INTERVAL 3000
+
 struct nfp_flower_service {
 	/** Flower service is enabled */
 	bool service_enabled;
+	/** Flower alarm is enabled */
+	bool alarm_enabled;
 	/** Flower service info */
 	struct nfp_service_info info;
 	/** Store flower cards' information */
@@ -33,6 +38,52 @@  nfp_flower_service_handle_get(struct nfp_net_hw_priv *hw_priv)
 	return hw_priv->pf_dev->process_share.fl_service;
 }
 
+static void
+nfp_flower_service_alarm_func(void *arg)
+{
+	int ret;
+	uint16_t slot;
+	struct nfp_net_hw_priv *hw_priv;
+	struct nfp_flower_service *service_handle;
+
+	service_handle = arg;
+	if (!service_handle->alarm_enabled)
+		goto alarm_set;
+
+	rte_spinlock_lock(&service_handle->spinlock);
+	for (slot = 0; slot < MAX_FLOWER_SERVICE_SLOT; slot++) {
+		hw_priv = service_handle->slots[slot];
+		if (hw_priv == NULL)
+			continue;
+
+		nfp_flower_ctrl_vnic_process(hw_priv);
+	}
+	rte_spinlock_unlock(&service_handle->spinlock);
+
+alarm_set:
+	ret = rte_eal_alarm_set(FLOWER_ALARM_INTERVAL, nfp_flower_service_alarm_func, arg);
+	if (ret < 0)
+		PMD_DRV_LOG(ERR, "Set flower service alarm failed.");
+}
+
+static int
+nfp_flower_service_alarm_enable(struct nfp_flower_service *service_handle)
+{
+	int ret;
+
+	ret = rte_eal_alarm_set(FLOWER_ALARM_INTERVAL, nfp_flower_service_alarm_func,
+			(void *)service_handle);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Flower service alarm initialization failed.");
+		return ret;
+	}
+
+	rte_spinlock_init(&service_handle->spinlock);
+	service_handle->alarm_enabled = true;
+
+	return 0;
+}
+
 static int
 nfp_flower_service_func(void *arg)
 {
@@ -109,11 +160,15 @@  nfp_flower_service_start(struct nfp_net_hw_priv *hw_priv)
 	}
 
 	/* Enable flower service when driver initializes the first NIC */
-	if (!service_handle->service_enabled) {
+	if (!service_handle->service_enabled && !service_handle->alarm_enabled) {
 		ret = nfp_flower_service_enable(service_handle);
 		if (ret != 0) {
-			PMD_DRV_LOG(ERR, "Could not enable flower service");
-			return -ESRCH;
+			PMD_DRV_LOG(INFO, "Could not enable flower service.");
+			ret = nfp_flower_service_alarm_enable(service_handle);
+			if (ret != 0) {
+				PMD_DRV_LOG(ERR, "Could not set flower service alarm.");
+				return ret;
+			}
 		}
 	}
 
@@ -157,8 +212,13 @@  nfp_flower_service_stop(struct nfp_net_hw_priv *hw_priv)
 	if (count > 1)
 		return;
 
-	if (nfp_service_disable(&service_handle->info) != 0)
-		PMD_DRV_LOG(ERR, "Could not disable service");
+	if (service_handle->service_enabled) {
+		if (nfp_service_disable(&service_handle->info) != 0)
+			PMD_DRV_LOG(ERR, "Could not disable service.");
+	} else if (service_handle->alarm_enabled) {
+		rte_eal_alarm_cancel(nfp_flower_service_alarm_func,
+				(void *)service_handle);
+	}
 }
 
 int