patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
@ 2023-04-24 14:30 Ivan Malov
  2023-04-24 14:30 ` [PATCH 2/2] net/sfc: invalidate dangling MAE flow action FW " Ivan Malov
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ivan Malov @ 2023-04-24 14:30 UTC (permalink / raw)
  To: dev
  Cc: Andrew Rybchenko, Ferruh Yigit, stable, Andy Moreton,
	Denis Pryazhennikov

When the client driver (the DPDK one, for instance) parses user flow
actions, it ends up with an action set specification. Next, in case
there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
the driver allocates these resources and indicates their IDs in the
action set specification. The API used to set these IDs checks that
the current value of the target ID is INVALID, prior to the call.

The latter check, however, prevents the driver from updating the
IDs on port restart. When the port goes down, the driver frees
the resources. When the port goes up, the driver reallocates
them, tries to set the IDs in the specification and fails.

In order to address the problem, add an API to drop the
current resource IDs in the actions set specification.

Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap action to a set")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Tested-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
---
 drivers/common/sfc_efx/base/efx.h      | 14 ++++++++++++++
 drivers/common/sfc_efx/base/efx_impl.h |  4 ++++
 drivers/common/sfc_efx/base/efx_mae.c  | 15 +++++++++++----
 drivers/common/sfc_efx/version.map     |  1 +
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 6028e08eb6..e4a5694ae2 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -4774,6 +4774,20 @@ efx_mae_action_set_fill_in_counter_id(
 	__in				efx_mae_actions_t *spec,
 	__in				const efx_counter_t *counter_idp);
 
+/*
+ * Clears dangling FW object IDs (counter ID, for instance) in
+ * the action set specification. Useful for adapter restarts,
+ * when all MAE objects need to be reallocated by the driver.
+ *
+ * This method only clears the IDs in the specification.
+ * The driver is still responsible for keeping the IDs
+ * separately and freeing them when stopping the port.
+ */
+LIBEFX_API
+extern					void
+efx_mae_action_set_clear_fw_rsrc_ids(
+	__in				efx_mae_actions_t *spec);
+
 /* Action set ID */
 typedef struct efx_mae_aset_id_s {
 	uint32_t id;
diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
index a48d4f6e04..bed4601715 100644
--- a/drivers/common/sfc_efx/base/efx_impl.h
+++ b/drivers/common/sfc_efx/base/efx_impl.h
@@ -1802,6 +1802,10 @@ typedef struct efx_mae_action_vlan_push_s {
 	uint16_t			emavp_tci_be;
 } efx_mae_action_vlan_push_t;
 
+/*
+ * Helper efx_mae_action_set_clear_fw_rsrc_ids() is responsible
+ * to initialise every field in this structure to INVALID value.
+ */
 typedef struct efx_mae_actions_rsrc_s {
 	efx_mae_mac_id_t		emar_dst_mac_id;
 	efx_mae_mac_id_t		emar_src_mac_id;
diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
index 7732d99992..4c33471f28 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -1394,10 +1394,7 @@ efx_mae_action_set_spec_init(
 		goto fail1;
 	}
 
-	spec->ema_rsrc.emar_dst_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_src_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_eh_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_counter_id.id = EFX_MAE_RSRC_ID_INVALID;
+	efx_mae_action_set_clear_fw_rsrc_ids(spec);
 
 	/*
 	 * Helpers which populate v2 actions must reject them when v2 is not
@@ -3027,6 +3024,16 @@ efx_mae_action_set_fill_in_counter_id(
 	return (rc);
 }
 
+					void
+efx_mae_action_set_clear_fw_rsrc_ids(
+	__in				efx_mae_actions_t *spec)
+{
+	spec->ema_rsrc.emar_dst_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_src_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_eh_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_counter_id.id = EFX_MAE_RSRC_ID_INVALID;
+}
+
 	__checkReturn			efx_rc_t
 efx_mae_counters_alloc(
 	__in				efx_nic_t *enp,
diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
index d36c3786fc..070de3ba54 100644
--- a/drivers/common/sfc_efx/version.map
+++ b/drivers/common/sfc_efx/version.map
@@ -90,6 +90,7 @@ INTERNAL {
 	efx_mae_action_rule_insert;
 	efx_mae_action_rule_remove;
 	efx_mae_action_set_alloc;
+	efx_mae_action_set_clear_fw_rsrc_ids;
 	efx_mae_action_set_fill_in_counter_id;
 	efx_mae_action_set_fill_in_dst_mac_id;
 	efx_mae_action_set_fill_in_eh_id;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] net/sfc: invalidate dangling MAE flow action FW resource IDs
  2023-04-24 14:30 [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs Ivan Malov
@ 2023-04-24 14:30 ` Ivan Malov
  2023-05-18 16:25 ` [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action " Ferruh Yigit
  2023-05-21  8:36 ` [PATCH v2] drivers: invalidate dangling MAE flow action FW " Ivan Malov
  2 siblings, 0 replies; 10+ messages in thread
From: Ivan Malov @ 2023-04-24 14:30 UTC (permalink / raw)
  To: dev
  Cc: Andrew Rybchenko, Ferruh Yigit, stable, Andy Moreton,
	Denis Pryazhennikov

When reinserting a flow (on port restart, for instance)
FW resource IDs found in the action set specification
need to be invalidated so that the new (reallocated)
FW resource IDs can be accepted by libefx again.

Fixes: 1bbd1ec2348a ("net/sfc: support action VXLAN encap in MAE backend")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Tested-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
---
 drivers/net/sfc/sfc_mae.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index e5e9257998..60b9fdc290 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -1180,6 +1180,8 @@ sfc_mae_action_set_disable(struct sfc_adapter *sa,
 	}
 
 	if (fw_rsrc->refcnt == 1) {
+		efx_mae_action_set_clear_fw_rsrc_ids(action_set->spec);
+
 		rc = efx_mae_action_set_free(sa->nic, &fw_rsrc->aset_id);
 		if (rc == 0) {
 			sfc_dbg(sa, "disabled action_set=%p with AS_ID=0x%08x",
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
  2023-04-24 14:30 [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs Ivan Malov
  2023-04-24 14:30 ` [PATCH 2/2] net/sfc: invalidate dangling MAE flow action FW " Ivan Malov
@ 2023-05-18 16:25 ` Ferruh Yigit
  2023-05-18 17:21   ` Ivan Malov
  2023-05-21  8:36 ` [PATCH v2] drivers: invalidate dangling MAE flow action FW " Ivan Malov
  2 siblings, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2023-05-18 16:25 UTC (permalink / raw)
  To: Ivan Malov, dev
  Cc: Andrew Rybchenko, stable, Andy Moreton, Denis Pryazhennikov

On 4/24/2023 3:30 PM, Ivan Malov wrote:
> When the client driver (the DPDK one, for instance) parses user flow
> actions, it ends up with an action set specification. Next, in case
> there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
> the driver allocates these resources and indicates their IDs in the
> action set specification. The API used to set these IDs checks that
> the current value of the target ID is INVALID, prior to the call.
> 
> The latter check, however, prevents the driver from updating the
> IDs on port restart. When the port goes down, the driver frees
> the resources. When the port goes up, the driver reallocates
> them, tries to set the IDs in the specification and fails.
> 
> In order to address the problem, add an API to drop the
> current resource IDs in the actions set specification.
> 
> Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap action to a set")
> Cc: stable@dpdk.org>

Hi Ivan,

As far as I understand this patch extracts some code into a function, so
I expect no functional change in this patch. So not clear what this
patch is fixing?

And I can see this new API is used in next patch and it fixes an issue.
But while backporting this fix to LTS we want both patch to backport
because there is a dependency.

If there is no functional change in this patch, what about merging these
two patches, and explain what is fixed? This also helps backporting.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
  2023-05-18 16:25 ` [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action " Ferruh Yigit
@ 2023-05-18 17:21   ` Ivan Malov
  2023-05-19  8:47     ` Ferruh Yigit
  0 siblings, 1 reply; 10+ messages in thread
From: Ivan Malov @ 2023-05-18 17:21 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: dev, Andrew Rybchenko, stable, Andy Moreton, Denis Pryazhennikov

Hi Ferruh,

Thanks for reviewing this. PSB.

On Thu, 18 May 2023, Ferruh Yigit wrote:

> On 4/24/2023 3:30 PM, Ivan Malov wrote:
>> When the client driver (the DPDK one, for instance) parses user flow
>> actions, it ends up with an action set specification. Next, in case
>> there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
>> the driver allocates these resources and indicates their IDs in the
>> action set specification. The API used to set these IDs checks that
>> the current value of the target ID is INVALID, prior to the call.
>>
>> The latter check, however, prevents the driver from updating the
>> IDs on port restart. When the port goes down, the driver frees
>> the resources. When the port goes up, the driver reallocates
>> them, tries to set the IDs in the specification and fails.
>>
>> In order to address the problem, add an API to drop the
>> current resource IDs in the actions set specification.
>>
>> Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap action to a set")
>> Cc: stable@dpdk.org>
>
> Hi Ivan,
>
> As far as I understand this patch extracts some code into a function, so
> I expect no functional change in this patch. So not clear what this
> patch is fixing?
>
> And I can see this new API is used in next patch and it fixes an issue.
> But while backporting this fix to LTS we want both patch to backport
> because there is a dependency.
>
> If there is no functional change in this patch, what about merging these
> two patches, and explain what is fixed? This also helps backporting.
>

As far as I know, changes to different trees (common/sfc_efx/base on the
one hand and drivers/net/sfc on the other) belong in separate patches.
Please correct me in case I've got that wrong.

Anyway, if it's not that hard to backport the two patches in their
current state, I vote for keeping them like that. I don't insist.

Thank you.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
  2023-05-18 17:21   ` Ivan Malov
@ 2023-05-19  8:47     ` Ferruh Yigit
  2023-05-19  9:01       ` Ivan Malov
  0 siblings, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2023-05-19  8:47 UTC (permalink / raw)
  To: Ivan Malov
  Cc: dev, Andrew Rybchenko, stable, Andy Moreton, Denis Pryazhennikov

On 5/18/2023 6:21 PM, Ivan Malov wrote:
> Hi Ferruh,
> 
> Thanks for reviewing this. PSB.
> 
> On Thu, 18 May 2023, Ferruh Yigit wrote:
> 
>> On 4/24/2023 3:30 PM, Ivan Malov wrote:
>>> When the client driver (the DPDK one, for instance) parses user flow
>>> actions, it ends up with an action set specification. Next, in case
>>> there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
>>> the driver allocates these resources and indicates their IDs in the
>>> action set specification. The API used to set these IDs checks that
>>> the current value of the target ID is INVALID, prior to the call.
>>>
>>> The latter check, however, prevents the driver from updating the
>>> IDs on port restart. When the port goes down, the driver frees
>>> the resources. When the port goes up, the driver reallocates
>>> them, tries to set the IDs in the specification and fails.
>>>
>>> In order to address the problem, add an API to drop the
>>> current resource IDs in the actions set specification.
>>>
>>> Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap
>>> action to a set")
>>> Cc: stable@dpdk.org>
>>
>> Hi Ivan,
>>
>> As far as I understand this patch extracts some code into a function, so
>> I expect no functional change in this patch. So not clear what this
>> patch is fixing?
>>
>> And I can see this new API is used in next patch and it fixes an issue.
>> But while backporting this fix to LTS we want both patch to backport
>> because there is a dependency.
>>
>> If there is no functional change in this patch, what about merging these
>> two patches, and explain what is fixed? This also helps backporting.
>>
> 
> As far as I know, changes to different trees (common/sfc_efx/base on the
> one hand and drivers/net/sfc on the other) belong in separate patches.
> Please correct me in case I've got that wrong.
> 
> Anyway, if it's not that hard to backport the two patches in their
> current state, I vote for keeping them like that. I don't insist.
> 

I think changes are simple and can go into single patch to help
dependency between two.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
  2023-05-19  8:47     ` Ferruh Yigit
@ 2023-05-19  9:01       ` Ivan Malov
  2023-05-22 10:21         ` Ferruh Yigit
  0 siblings, 1 reply; 10+ messages in thread
From: Ivan Malov @ 2023-05-19  9:01 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: dev, Andrew Rybchenko, stable, Andy Moreton, Denis Pryazhennikov

Hi Ferruh,

On Fri, 19 May 2023, Ferruh Yigit wrote:

> On 5/18/2023 6:21 PM, Ivan Malov wrote:
>> Hi Ferruh,
>>
>> Thanks for reviewing this. PSB.
>>
>> On Thu, 18 May 2023, Ferruh Yigit wrote:
>>
>>> On 4/24/2023 3:30 PM, Ivan Malov wrote:
>>>> When the client driver (the DPDK one, for instance) parses user flow
>>>> actions, it ends up with an action set specification. Next, in case
>>>> there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
>>>> the driver allocates these resources and indicates their IDs in the
>>>> action set specification. The API used to set these IDs checks that
>>>> the current value of the target ID is INVALID, prior to the call.
>>>>
>>>> The latter check, however, prevents the driver from updating the
>>>> IDs on port restart. When the port goes down, the driver frees
>>>> the resources. When the port goes up, the driver reallocates
>>>> them, tries to set the IDs in the specification and fails.
>>>>
>>>> In order to address the problem, add an API to drop the
>>>> current resource IDs in the actions set specification.
>>>>
>>>> Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap
>>>> action to a set")
>>>> Cc: stable@dpdk.org>
>>>
>>> Hi Ivan,
>>>
>>> As far as I understand this patch extracts some code into a function, so
>>> I expect no functional change in this patch. So not clear what this
>>> patch is fixing?
>>>
>>> And I can see this new API is used in next patch and it fixes an issue.
>>> But while backporting this fix to LTS we want both patch to backport
>>> because there is a dependency.
>>>
>>> If there is no functional change in this patch, what about merging these
>>> two patches, and explain what is fixed? This also helps backporting.
>>>
>>
>> As far as I know, changes to different trees (common/sfc_efx/base on the
>> one hand and drivers/net/sfc on the other) belong in separate patches.
>> Please correct me in case I've got that wrong.
>>
>> Anyway, if it's not that hard to backport the two patches in their
>> current state, I vote for keeping them like that. I don't insist.
>>
>
> I think changes are simple and can go into single patch to help
> dependency between two.
>
>
Thanks for perseverance. Now you mention it, what prefix in the summary
should I use? Just "drivers: "? For the unified patch, that is.

Ivan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] drivers: invalidate dangling MAE flow action FW resource IDs
  2023-04-24 14:30 [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs Ivan Malov
  2023-04-24 14:30 ` [PATCH 2/2] net/sfc: invalidate dangling MAE flow action FW " Ivan Malov
  2023-05-18 16:25 ` [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action " Ferruh Yigit
@ 2023-05-21  8:36 ` Ivan Malov
  2023-05-22  6:48   ` Andrew Rybchenko
  2 siblings, 1 reply; 10+ messages in thread
From: Ivan Malov @ 2023-05-21  8:36 UTC (permalink / raw)
  To: dev
  Cc: Andrew Rybchenko, Ferruh Yigit, stable, Andy Moreton,
	Denis Pryazhennikov

When reinserting a flow (on port restart, for instance)
FW resource IDs found in the action set specification
need to be invalidated so that the new (reallocated)
FW resource IDs can be accepted by libefx again.

Fixes: 1bbd1ec2348a ("net/sfc: support action VXLAN encap in MAE backend")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Tested-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
---
v2: squashed 1/2 and 2/2 of the previous version as per Ferruh's request

 drivers/common/sfc_efx/base/efx.h      | 14 ++++++++++++++
 drivers/common/sfc_efx/base/efx_impl.h |  4 ++++
 drivers/common/sfc_efx/base/efx_mae.c  | 15 +++++++++++----
 drivers/common/sfc_efx/version.map     |  1 +
 drivers/net/sfc/sfc_mae.c              |  2 ++
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index f4fa88f169..49e29dcc1c 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -4748,6 +4748,20 @@ efx_mae_action_set_fill_in_counter_id(
 	__in				efx_mae_actions_t *spec,
 	__in				const efx_counter_t *counter_idp);
 
+/*
+ * Clears dangling FW object IDs (counter ID, for instance) in
+ * the action set specification. Useful for adapter restarts,
+ * when all MAE objects need to be reallocated by the driver.
+ *
+ * This method only clears the IDs in the specification.
+ * The driver is still responsible for keeping the IDs
+ * separately and freeing them when stopping the port.
+ */
+LIBEFX_API
+extern					void
+efx_mae_action_set_clear_fw_rsrc_ids(
+	__in				efx_mae_actions_t *spec);
+
 /* Action set ID */
 typedef struct efx_mae_aset_id_s {
 	uint32_t id;
diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
index 9a5d465fa0..45e99d01c5 100644
--- a/drivers/common/sfc_efx/base/efx_impl.h
+++ b/drivers/common/sfc_efx/base/efx_impl.h
@@ -1800,6 +1800,10 @@ typedef struct efx_mae_action_vlan_push_s {
 	uint16_t			emavp_tci_be;
 } efx_mae_action_vlan_push_t;
 
+/*
+ * Helper efx_mae_action_set_clear_fw_rsrc_ids() is responsible
+ * to initialise every field in this structure to INVALID value.
+ */
 typedef struct efx_mae_actions_rsrc_s {
 	efx_mae_mac_id_t		emar_dst_mac_id;
 	efx_mae_mac_id_t		emar_src_mac_id;
diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
index 7732d99992..4c33471f28 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -1394,10 +1394,7 @@ efx_mae_action_set_spec_init(
 		goto fail1;
 	}
 
-	spec->ema_rsrc.emar_dst_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_src_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_eh_id.id = EFX_MAE_RSRC_ID_INVALID;
-	spec->ema_rsrc.emar_counter_id.id = EFX_MAE_RSRC_ID_INVALID;
+	efx_mae_action_set_clear_fw_rsrc_ids(spec);
 
 	/*
 	 * Helpers which populate v2 actions must reject them when v2 is not
@@ -3027,6 +3024,16 @@ efx_mae_action_set_fill_in_counter_id(
 	return (rc);
 }
 
+					void
+efx_mae_action_set_clear_fw_rsrc_ids(
+	__in				efx_mae_actions_t *spec)
+{
+	spec->ema_rsrc.emar_dst_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_src_mac_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_eh_id.id = EFX_MAE_RSRC_ID_INVALID;
+	spec->ema_rsrc.emar_counter_id.id = EFX_MAE_RSRC_ID_INVALID;
+}
+
 	__checkReturn			efx_rc_t
 efx_mae_counters_alloc(
 	__in				efx_nic_t *enp,
diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
index aabc354118..d9b04a611d 100644
--- a/drivers/common/sfc_efx/version.map
+++ b/drivers/common/sfc_efx/version.map
@@ -89,6 +89,7 @@ INTERNAL {
 	efx_mae_action_rule_insert;
 	efx_mae_action_rule_remove;
 	efx_mae_action_set_alloc;
+	efx_mae_action_set_clear_fw_rsrc_ids;
 	efx_mae_action_set_fill_in_counter_id;
 	efx_mae_action_set_fill_in_dst_mac_id;
 	efx_mae_action_set_fill_in_eh_id;
diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index e5e9257998..60b9fdc290 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -1180,6 +1180,8 @@ sfc_mae_action_set_disable(struct sfc_adapter *sa,
 	}
 
 	if (fw_rsrc->refcnt == 1) {
+		efx_mae_action_set_clear_fw_rsrc_ids(action_set->spec);
+
 		rc = efx_mae_action_set_free(sa->nic, &fw_rsrc->aset_id);
 		if (rc == 0) {
 			sfc_dbg(sa, "disabled action_set=%p with AS_ID=0x%08x",
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drivers: invalidate dangling MAE flow action FW resource IDs
  2023-05-21  8:36 ` [PATCH v2] drivers: invalidate dangling MAE flow action FW " Ivan Malov
@ 2023-05-22  6:48   ` Andrew Rybchenko
  2023-05-22 10:20     ` Ferruh Yigit
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Rybchenko @ 2023-05-22  6:48 UTC (permalink / raw)
  To: Ivan Malov, dev; +Cc: Ferruh Yigit, stable, Andy Moreton, Denis Pryazhennikov

On 5/21/23 11:36, Ivan Malov wrote:
> When reinserting a flow (on port restart, for instance)
> FW resource IDs found in the action set specification
> need to be invalidated so that the new (reallocated)
> FW resource IDs can be accepted by libefx again.
> 
> Fixes: 1bbd1ec2348a ("net/sfc: support action VXLAN encap in MAE backend")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Tested-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drivers: invalidate dangling MAE flow action FW resource IDs
  2023-05-22  6:48   ` Andrew Rybchenko
@ 2023-05-22 10:20     ` Ferruh Yigit
  0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2023-05-22 10:20 UTC (permalink / raw)
  To: Andrew Rybchenko, Ivan Malov, dev
  Cc: stable, Andy Moreton, Denis Pryazhennikov

On 5/22/2023 7:48 AM, Andrew Rybchenko wrote:
> On 5/21/23 11:36, Ivan Malov wrote:
>> When reinserting a flow (on port restart, for instance)
>> FW resource IDs found in the action set specification
>> need to be invalidated so that the new (reallocated)
>> FW resource IDs can be accepted by libefx again.
>>
>> Fixes: 1bbd1ec2348a ("net/sfc: support action VXLAN encap in MAE
>> backend")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> Tested-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> 

Applied to dpdk-next-net/main, thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs
  2023-05-19  9:01       ` Ivan Malov
@ 2023-05-22 10:21         ` Ferruh Yigit
  0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2023-05-22 10:21 UTC (permalink / raw)
  To: Ivan Malov
  Cc: dev, Andrew Rybchenko, stable, Andy Moreton, Denis Pryazhennikov

On 5/19/2023 10:01 AM, Ivan Malov wrote:
> Hi Ferruh,
> 
> On Fri, 19 May 2023, Ferruh Yigit wrote:
> 
>> On 5/18/2023 6:21 PM, Ivan Malov wrote:
>>> Hi Ferruh,
>>>
>>> Thanks for reviewing this. PSB.
>>>
>>> On Thu, 18 May 2023, Ferruh Yigit wrote:
>>>
>>>> On 4/24/2023 3:30 PM, Ivan Malov wrote:
>>>>> When the client driver (the DPDK one, for instance) parses user flow
>>>>> actions, it ends up with an action set specification. Next, in case
>>>>> there are any FW resource-backed actions, like COUNT or SET_DST_MAC,
>>>>> the driver allocates these resources and indicates their IDs in the
>>>>> action set specification. The API used to set these IDs checks that
>>>>> the current value of the target ID is INVALID, prior to the call.
>>>>>
>>>>> The latter check, however, prevents the driver from updating the
>>>>> IDs on port restart. When the port goes down, the driver frees
>>>>> the resources. When the port goes up, the driver reallocates
>>>>> them, tries to set the IDs in the specification and fails.
>>>>>
>>>>> In order to address the problem, add an API to drop the
>>>>> current resource IDs in the actions set specification.
>>>>>
>>>>> Fixes: 3907defa5bf0 ("common/sfc_efx/base: support adding encap
>>>>> action to a set")
>>>>> Cc: stable@dpdk.org>
>>>>
>>>> Hi Ivan,
>>>>
>>>> As far as I understand this patch extracts some code into a
>>>> function, so
>>>> I expect no functional change in this patch. So not clear what this
>>>> patch is fixing?
>>>>
>>>> And I can see this new API is used in next patch and it fixes an issue.
>>>> But while backporting this fix to LTS we want both patch to backport
>>>> because there is a dependency.
>>>>
>>>> If there is no functional change in this patch, what about merging
>>>> these
>>>> two patches, and explain what is fixed? This also helps backporting.
>>>>
>>>
>>> As far as I know, changes to different trees (common/sfc_efx/base on the
>>> one hand and drivers/net/sfc on the other) belong in separate patches.
>>> Please correct me in case I've got that wrong.
>>>
>>> Anyway, if it's not that hard to backport the two patches in their
>>> current state, I vote for keeping them like that. I don't insist.
>>>
>>
>> I think changes are simple and can go into single patch to help
>> dependency between two.
>>
>>
> Thanks for perseverance. Now you mention it, what prefix in the summary
> should I use? Just "drivers: "? For the unified patch, that is.
> 

updated as 'net/sfc: ...' while merging.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-05-22 10:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-24 14:30 [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action resource IDs Ivan Malov
2023-04-24 14:30 ` [PATCH 2/2] net/sfc: invalidate dangling MAE flow action FW " Ivan Malov
2023-05-18 16:25 ` [PATCH 1/2] common/sfc_efx/base: add API to drop MAE action " Ferruh Yigit
2023-05-18 17:21   ` Ivan Malov
2023-05-19  8:47     ` Ferruh Yigit
2023-05-19  9:01       ` Ivan Malov
2023-05-22 10:21         ` Ferruh Yigit
2023-05-21  8:36 ` [PATCH v2] drivers: invalidate dangling MAE flow action FW " Ivan Malov
2023-05-22  6:48   ` Andrew Rybchenko
2023-05-22 10:20     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).