DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <arybchenko@solarflare.com>
To: <dev@dpdk.org>
Cc: Igor Romanov <igor.romanov@oktetlabs.ru>, <stable@dpdk.org>
Subject: [dpdk-dev] [PATCH 5/5] net/sfc/base: handle manual and auto filter clashes in EF10
Date: Tue, 10 Mar 2020 09:48:39 +0000	[thread overview]
Message-ID: <1583833719-6597-6-git-send-email-arybchenko@solarflare.com> (raw)
In-Reply-To: <1583833719-6597-1-git-send-email-arybchenko@solarflare.com>

From: Igor Romanov <igor.romanov@oktetlabs.ru>

Make user filters a priority in EF10 datapath. When a manual
filter with a specification that is equal to an existing auto
filter is inserted, the manual filter:
- replaces auto filter if the specification is exclusive;
- is inserted along existing auto filter otherwise;

In the first case the auto filter that was replaced is saved.
This saved filter can be updated on filter reconfiguration and
is restored on the manual filter removal.

Fixes: e7cd430c864f ("net/sfc/base: import SFN7xxx family support")
Cc: stable@dpdk.org

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 513 ++++++++++++++++++++++-------
 drivers/net/sfc/base/ef10_impl.h   |   4 +-
 drivers/net/sfc/base/efx.h         |   7 +
 drivers/net/sfc/base/efx_filter.c  |  17 +-
 drivers/net/sfc/base/efx_impl.h    |  21 +-
 5 files changed, 434 insertions(+), 128 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 5578765ab..597ec3493 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -590,6 +590,231 @@ ef10_filter_restore(
 	return (rc);
 }
 
+enum ef10_filter_add_action_e {
+	/* Insert a new filter */
+	EF10_FILTER_ADD_NEW,
+	/*
+	 * Replace old filter with a new, overriding the old one
+	 * if it has lower priority.
+	 */
+	EF10_FILTER_ADD_REPLACE,
+	/* Store new, lower priority filter as overridden by old filter */
+	EF10_FILTER_ADD_STORE,
+	/* Special case for AUTO filters, remove AUTO_OLD flag */
+	EF10_FILTER_ADD_REFRESH,
+};
+
+static	__checkReturn	efx_rc_t
+ef10_filter_add_lookup_equal_spec(
+	__in		efx_filter_spec_t *spec,
+	__in		efx_filter_spec_t *probe_spec,
+	__in		efx_filter_replacement_policy_t policy,
+	__out		boolean_t *found)
+{
+	efx_rc_t rc;
+
+	/* Refreshing AUTO filter */
+	if (spec->efs_priority == EFX_FILTER_PRI_AUTO &&
+	    probe_spec->efs_priority == EFX_FILTER_PRI_AUTO) {
+		*found = B_TRUE;
+		return (0);
+	}
+
+	/*
+	 * With exclusive filters, higher priority ones
+	 * override lower priority ones, and lower priority
+	 * ones are stored in case the higher priority one
+	 * is removed.
+	 */
+	if (ef10_filter_is_exclusive(spec)) {
+		switch (policy) {
+		case EFX_FILTER_REPLACEMENT_HIGHER_OR_EQUAL_PRIORITY:
+			if (spec->efs_priority == probe_spec->efs_priority) {
+				*found = B_TRUE;
+				break;
+			}
+			/* Fall-through */
+		case EFX_FILTER_REPLACEMENT_HIGHER_PRIORITY:
+			if (spec->efs_priority > probe_spec->efs_priority) {
+				*found = B_TRUE;
+				break;
+			}
+			/* Fall-through */
+		case EFX_FILTER_REPLACEMENT_NEVER:
+			/*
+			 * Lower priority filter needs to be
+			 * stored. It does *not* replace the
+			 * old one. That is why EEXIST is not
+			 * returned in that case.
+			 */
+			if (spec->efs_priority < probe_spec->efs_priority) {
+				*found = B_TRUE;
+				break;
+			} else {
+				rc = EEXIST;
+				goto fail1;
+			}
+		default:
+			EFSYS_ASSERT(0);
+			rc = EEXIST;
+			goto fail2;
+		}
+	} else {
+		*found = B_FALSE;
+	}
+
+	return (0);
+
+fail2:
+	EFSYS_PROBE(fail2);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
+
+static			void
+ef10_filter_add_select_action(
+	__in		efx_filter_spec_t *saved_spec,
+	__in		efx_filter_spec_t *spec,
+	__out		enum ef10_filter_add_action_e *action,
+	__out		efx_filter_spec_t **overridden_spec)
+{
+	efx_filter_spec_t *overridden = NULL;
+
+	if (saved_spec == NULL) {
+		*action = EF10_FILTER_ADD_NEW;
+	} else if (ef10_filter_is_exclusive(spec) == B_FALSE) {
+		/*
+		 * Non-exclusive filters are always stored in separate entries
+		 * in the table. The only case involving a saved spec is
+		 * refreshing an AUTO filter.
+		 */
+		EFSYS_ASSERT(saved_spec->efs_overridden_spec == NULL);
+		EFSYS_ASSERT(spec->efs_priority == EFX_FILTER_PRI_AUTO);
+		EFSYS_ASSERT(saved_spec->efs_priority == EFX_FILTER_PRI_AUTO);
+		*action = EF10_FILTER_ADD_REFRESH;
+	} else {
+		/* Exclusive filters stored in the same entry */
+		if (spec->efs_priority > saved_spec->efs_priority) {
+			/*
+			 * Insert a high priority filter over a lower priority
+			 * one. Only two priority levels are implemented, so
+			 * there must not already be an overridden filter.
+			 */
+			EFX_STATIC_ASSERT(EFX_FILTER_NPRI == 2);
+			EFSYS_ASSERT(saved_spec->efs_overridden_spec == NULL);
+			overridden = saved_spec;
+			*action = EF10_FILTER_ADD_REPLACE;
+		} else if (spec->efs_priority == saved_spec->efs_priority) {
+			/* Replace in-place or refresh an existing filter */
+			if (spec->efs_priority == EFX_FILTER_PRI_AUTO)
+				*action = EF10_FILTER_ADD_REFRESH;
+			else
+				*action = EF10_FILTER_ADD_REPLACE;
+		} else {
+			/*
+			 * Insert a lower priority filter, storing it in case
+			 * the higher priority filter is removed.
+			 *
+			 * Currently there are only two priority levels, so this
+			 * must be an AUTO filter.
+			 */
+			EFX_STATIC_ASSERT(EFX_FILTER_NPRI == 2);
+			EFSYS_ASSERT(spec->efs_priority == EFX_FILTER_PRI_AUTO);
+			if (saved_spec->efs_overridden_spec != NULL) {
+				*action = EF10_FILTER_ADD_REFRESH;
+			} else {
+				overridden = spec;
+				*action = EF10_FILTER_ADD_STORE;
+			}
+		}
+	}
+
+	*overridden_spec = overridden;
+}
+
+static	__checkReturn	efx_rc_t
+ef10_filter_add_execute_action(
+	__in		efx_nic_t *enp,
+	__in		efx_filter_spec_t *saved_spec,
+	__in		efx_filter_spec_t *spec,
+	__in		efx_filter_spec_t *overridden_spec,
+	__in		enum ef10_filter_add_action_e action,
+	__in		int ins_index)
+{
+	ef10_filter_table_t *eftp = enp->en_filter.ef_ef10_filter_table;
+	efsys_lock_state_t state;
+	efx_rc_t rc;
+
+	EFSYS_LOCK(enp->en_eslp, state);
+
+	if (action == EF10_FILTER_ADD_REFRESH) {
+		ef10_filter_set_entry_not_auto_old(eftp, ins_index);
+		goto out_unlock;
+	} else if (action == EF10_FILTER_ADD_STORE) {
+		EFSYS_ASSERT(overridden_spec != NULL);
+		saved_spec->efs_overridden_spec = overridden_spec;
+		goto out_unlock;
+	}
+
+	EFSYS_UNLOCK(enp->en_eslp, state);
+
+	switch (action) {
+	case EF10_FILTER_ADD_REPLACE:
+		/*
+		 * On replacing the filter handle may change after a
+		 * successful replace operation.
+		 */
+		rc = efx_mcdi_filter_op_add(enp, spec,
+		    MC_CMD_FILTER_OP_IN_OP_REPLACE,
+		    &eftp->eft_entry[ins_index].efe_handle);
+		break;
+	case EF10_FILTER_ADD_NEW:
+		if (ef10_filter_is_exclusive(spec)) {
+			rc = efx_mcdi_filter_op_add(enp, spec,
+			    MC_CMD_FILTER_OP_IN_OP_INSERT,
+			    &eftp->eft_entry[ins_index].efe_handle);
+		} else {
+			rc = efx_mcdi_filter_op_add(enp, spec,
+			    MC_CMD_FILTER_OP_IN_OP_SUBSCRIBE,
+			    &eftp->eft_entry[ins_index].efe_handle);
+		}
+		break;
+	default:
+		rc = EINVAL;
+		EFSYS_ASSERT(0);
+		break;
+	}
+	if (rc != 0)
+		goto fail1;
+
+	EFSYS_LOCK(enp->en_eslp, state);
+
+	if (action == EF10_FILTER_ADD_REPLACE) {
+		/* Update the fields that may differ */
+		saved_spec->efs_priority = spec->efs_priority;
+		saved_spec->efs_flags = spec->efs_flags;
+		saved_spec->efs_rss_context = spec->efs_rss_context;
+		saved_spec->efs_dmaq_id = spec->efs_dmaq_id;
+
+		if (overridden_spec != NULL)
+			saved_spec->efs_overridden_spec = overridden_spec;
+	}
+
+out_unlock:
+	EFSYS_UNLOCK(enp->en_eslp, state);
+
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 /*
  * An arbitrary search limit for the software hash table. As per the linux net
  * driver.
@@ -600,22 +825,24 @@ static	__checkReturn	efx_rc_t
 ef10_filter_add_internal(
 	__in		efx_nic_t *enp,
 	__inout		efx_filter_spec_t *spec,
-	__in		boolean_t may_replace,
+	__in		efx_filter_replacement_policy_t policy,
 	__out_opt	uint32_t *filter_id)
 {
 	efx_rc_t rc;
 	ef10_filter_table_t *eftp = enp->en_filter.ef_ef10_filter_table;
+	enum ef10_filter_add_action_e action;
+	efx_filter_spec_t *overridden_spec = NULL;
 	efx_filter_spec_t *saved_spec;
 	uint32_t hash;
 	unsigned int depth;
 	int ins_index;
-	boolean_t replacing = B_FALSE;
-	unsigned int i;
 	efsys_lock_state_t state;
 	boolean_t locked = B_FALSE;
 
 	EFSYS_ASSERT(EFX_FAMILY_IS_EF10(enp));
 
+	EFSYS_ASSERT(spec->efs_overridden_spec == NULL);
+
 	hash = ef10_filter_hash(spec);
 
 	/*
@@ -635,35 +862,33 @@ ef10_filter_add_internal(
 	ins_index = -1;
 
 	for (depth = 1; depth <= EF10_FILTER_SEARCH_LIMIT; depth++) {
-		i = (hash + depth) & (EFX_EF10_FILTER_TBL_ROWS - 1);
-		saved_spec = ef10_filter_entry_spec(eftp, i);
+		unsigned int probe_index;
+		efx_filter_spec_t *probe_spec;
 
-		if (saved_spec == NULL) {
+		probe_index = (hash + depth) & (EFX_EF10_FILTER_TBL_ROWS - 1);
+		probe_spec = ef10_filter_entry_spec(eftp, probe_index);
+
+		if (probe_spec == NULL) {
 			if (ins_index < 0)
-				ins_index = i;
-		} else if (ef10_filter_equal(spec, saved_spec)) {
-			if (ef10_filter_entry_is_busy(eftp, i)) {
+				ins_index = probe_index;
+		} else if (ef10_filter_equal(spec, probe_spec)) {
+			boolean_t found;
+
+			if (ef10_filter_entry_is_busy(eftp, probe_index)) {
 				EFSYS_UNLOCK(enp->en_eslp, state);
 				locked = B_FALSE;
 				goto retry;
 			}
 
-			if (saved_spec->efs_priority == EFX_FILTER_PRI_AUTO) {
-				ins_index = i;
-				goto found;
-			}
+			rc = ef10_filter_add_lookup_equal_spec(spec,
+			    probe_spec, policy, &found);
+			if (rc != 0)
+				goto fail1;
 
-			if (ef10_filter_is_exclusive(spec)) {
-				if (may_replace) {
-					ins_index = i;
-					goto found;
-				} else {
-					rc = EEXIST;
-					goto fail1;
-				}
+			if (found != B_FALSE) {
+				ins_index = probe_index;
+				break;
 			}
-
-			/* Leave existing */
 		}
 	}
 
@@ -676,93 +901,90 @@ ef10_filter_add_internal(
 		goto fail2;
 	}
 
-found:
 	/*
-	 * Create a software table entry if necessary, and mark it
-	 * busy.  We might yet fail to insert, but any attempt to
-	 * insert a conflicting filter while we're waiting for the
-	 * firmware must find the busy entry.
+	 * Mark software table entry busy. We might yet fail to insert,
+	 * but any attempt to insert a conflicting filter while we're
+	 * waiting for the firmware must find the busy entry.
 	 */
+	ef10_filter_set_entry_busy(eftp, ins_index);
+
 	saved_spec = ef10_filter_entry_spec(eftp, ins_index);
-	if (saved_spec) {
-		if (saved_spec->efs_priority == EFX_FILTER_PRI_AUTO) {
-			/* This is a filter we are refreshing */
-			ef10_filter_set_entry_not_auto_old(eftp, ins_index);
-			goto out_unlock;
+	ef10_filter_add_select_action(saved_spec, spec, &action,
+	    &overridden_spec);
 
-		}
-		replacing = B_TRUE;
-	} else {
-		EFSYS_KMEM_ALLOC(enp->en_esip, sizeof (*spec), saved_spec);
-		if (!saved_spec) {
+	/*
+	 * Allocate a new filter if found entry is empty or
+	 * a filter should be overridden.
+	 */
+	if (overridden_spec != NULL || saved_spec == NULL) {
+		efx_filter_spec_t *new_spec;
+
+		EFSYS_UNLOCK(enp->en_eslp, state);
+		locked = B_FALSE;
+
+		EFSYS_KMEM_ALLOC(enp->en_esip, sizeof (*new_spec), new_spec);
+		if (new_spec == NULL) {
 			rc = ENOMEM;
+			overridden_spec = NULL;
 			goto fail3;
 		}
-		*saved_spec = *spec;
-		ef10_filter_set_entry(eftp, ins_index, saved_spec);
+
+		EFSYS_LOCK(enp->en_eslp, state);
+		locked = B_TRUE;
+
+		if (saved_spec == NULL) {
+			*new_spec = *spec;
+			ef10_filter_set_entry(eftp, ins_index, new_spec);
+		} else {
+			*new_spec = *overridden_spec;
+			overridden_spec = new_spec;
+		}
 	}
-	ef10_filter_set_entry_busy(eftp, ins_index);
 
 	EFSYS_UNLOCK(enp->en_eslp, state);
 	locked = B_FALSE;
 
-	/*
-	 * On replacing the filter handle may change after after a successful
-	 * replace operation.
-	 */
-	if (replacing) {
-		rc = efx_mcdi_filter_op_add(enp, spec,
-		    MC_CMD_FILTER_OP_IN_OP_REPLACE,
-		    &eftp->eft_entry[ins_index].efe_handle);
-	} else if (ef10_filter_is_exclusive(spec)) {
-		rc = efx_mcdi_filter_op_add(enp, spec,
-		    MC_CMD_FILTER_OP_IN_OP_INSERT,
-		    &eftp->eft_entry[ins_index].efe_handle);
-	} else {
-		rc = efx_mcdi_filter_op_add(enp, spec,
-		    MC_CMD_FILTER_OP_IN_OP_SUBSCRIBE,
-		    &eftp->eft_entry[ins_index].efe_handle);
-	}
-
+	rc = ef10_filter_add_execute_action(enp, saved_spec, spec,
+	    overridden_spec, action, ins_index);
 	if (rc != 0)
 		goto fail4;
 
-	EFSYS_LOCK(enp->en_eslp, state);
-	locked = B_TRUE;
-
-	if (replacing) {
-		/* Update the fields that may differ */
-		saved_spec->efs_priority = spec->efs_priority;
-		saved_spec->efs_flags = spec->efs_flags;
-		saved_spec->efs_rss_context = spec->efs_rss_context;
-		saved_spec->efs_dmaq_id = spec->efs_dmaq_id;
-	}
+	if (filter_id)
+		*filter_id = ins_index;
 
+	EFSYS_LOCK(enp->en_eslp, state);
 	ef10_filter_set_entry_not_busy(eftp, ins_index);
-
-out_unlock:
-
 	EFSYS_UNLOCK(enp->en_eslp, state);
-	locked = B_FALSE;
-
-	if (filter_id)
-		*filter_id = ins_index;
 
 	return (0);
 
 fail4:
 	EFSYS_PROBE(fail4);
 
-	if (!replacing) {
-		EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec), saved_spec);
-		saved_spec = NULL;
+	EFSYS_ASSERT(locked == B_FALSE);
+	EFSYS_LOCK(enp->en_eslp, state);
+
+	if (action == EF10_FILTER_ADD_NEW) {
+		EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec),
+		    ef10_filter_entry_spec(eftp, ins_index));
+		ef10_filter_set_entry(eftp, ins_index, NULL);
 	}
-	ef10_filter_set_entry_not_busy(eftp, ins_index);
-	ef10_filter_set_entry(eftp, ins_index, NULL);
+
+	EFSYS_UNLOCK(enp->en_eslp, state);
+
+	if (overridden_spec != NULL)
+		EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec), overridden_spec);
 
 fail3:
 	EFSYS_PROBE(fail3);
 
+	EFSYS_ASSERT(locked == B_FALSE);
+	EFSYS_LOCK(enp->en_eslp, state);
+
+	ef10_filter_set_entry_not_busy(eftp, ins_index);
+
+	EFSYS_UNLOCK(enp->en_eslp, state);
+
 fail2:
 	EFSYS_PROBE(fail2);
 
@@ -779,11 +1001,11 @@ ef10_filter_add_internal(
 ef10_filter_add(
 	__in		efx_nic_t *enp,
 	__inout		efx_filter_spec_t *spec,
-	__in		boolean_t may_replace)
+	__in		enum efx_filter_replacement_policy_e policy)
 {
 	efx_rc_t rc;
 
-	rc = ef10_filter_add_internal(enp, spec, may_replace, NULL);
+	rc = ef10_filter_add_internal(enp, spec, policy, NULL);
 	if (rc != 0)
 		goto fail1;
 
@@ -795,11 +1017,15 @@ ef10_filter_add(
 	return (rc);
 }
 
-
+/*
+ * Delete a filter by index from the filter table with priority
+ * that is not higher than specified.
+ */
 static	__checkReturn	efx_rc_t
 ef10_filter_delete_internal(
 	__in		efx_nic_t *enp,
-	__in		uint32_t filter_id)
+	__in		uint32_t filter_id,
+	__in		efx_filter_priority_t priority)
 {
 	efx_rc_t rc;
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
@@ -821,7 +1047,8 @@ ef10_filter_delete_internal(
 		EFSYS_LOCK(enp->en_eslp, state);
 	}
 	if ((spec = ef10_filter_entry_spec(table, filter_idx)) != NULL) {
-		ef10_filter_set_entry_busy(table, filter_idx);
+		if (spec->efs_priority <= priority)
+			ef10_filter_set_entry_busy(table, filter_idx);
 	}
 	EFSYS_UNLOCK(enp->en_eslp, state);
 
@@ -830,31 +1057,53 @@ ef10_filter_delete_internal(
 		goto fail1;
 	}
 
-	/*
-	 * Try to remove the hardware filter. This may fail if the MC has
-	 * rebooted (which frees all hardware filter resources).
-	 */
-	if (ef10_filter_is_exclusive(spec)) {
-		rc = efx_mcdi_filter_op_delete(enp,
-		    MC_CMD_FILTER_OP_IN_OP_REMOVE,
-		    &table->eft_entry[filter_idx].efe_handle);
+	if (spec->efs_priority > priority) {
+		/*
+		 * Applied filter stays, but overriden filter is removed since
+		 * next user request to delete the applied filter should not
+		 * restore outdated filter.
+		 */
+		if (spec->efs_overridden_spec != NULL) {
+			EFSYS_ASSERT(spec->efs_overridden_spec->efs_overridden_spec ==
+			    NULL);
+			EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec),
+			    spec->efs_overridden_spec);
+			spec->efs_overridden_spec = NULL;
+		}
 	} else {
-		rc = efx_mcdi_filter_op_delete(enp,
-		    MC_CMD_FILTER_OP_IN_OP_UNSUBSCRIBE,
-		    &table->eft_entry[filter_idx].efe_handle);
-	}
+		/*
+		 * Try to remove the hardware filter or replace it with the
+		 * saved automatic filter. This may fail if the MC has
+		 * rebooted (which frees all hardware filter resources).
+		 */
+		if (spec->efs_overridden_spec != NULL) {
+			rc = efx_mcdi_filter_op_add(enp,
+			    spec->efs_overridden_spec,
+			    MC_CMD_FILTER_OP_IN_OP_REPLACE,
+			    &table->eft_entry[filter_idx].efe_handle);
+		} else if (ef10_filter_is_exclusive(spec)) {
+			rc = efx_mcdi_filter_op_delete(enp,
+			    MC_CMD_FILTER_OP_IN_OP_REMOVE,
+			    &table->eft_entry[filter_idx].efe_handle);
+		} else {
+			rc = efx_mcdi_filter_op_delete(enp,
+			    MC_CMD_FILTER_OP_IN_OP_UNSUBSCRIBE,
+			    &table->eft_entry[filter_idx].efe_handle);
+		}
 
-	/* Free the software table entry */
-	EFSYS_LOCK(enp->en_eslp, state);
-	ef10_filter_set_entry_not_busy(table, filter_idx);
-	ef10_filter_set_entry(table, filter_idx, NULL);
-	EFSYS_UNLOCK(enp->en_eslp, state);
+		/* Free the software table entry */
+		EFSYS_LOCK(enp->en_eslp, state);
+		ef10_filter_set_entry_not_busy(table, filter_idx);
+		ef10_filter_set_entry(table, filter_idx,
+		    spec->efs_overridden_spec);
+		EFSYS_UNLOCK(enp->en_eslp, state);
 
-	EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec), spec);
+		EFSYS_KMEM_FREE(enp->en_esip, sizeof (*spec), spec);
 
-	/* Check result of hardware filter removal */
-	if (rc != 0)
-		goto fail2;
+		/* Check result of hardware filter removal */
+		if (rc != 0)
+			goto fail2;
+	}
 
 	return (0);
 
@@ -867,6 +1116,25 @@ ef10_filter_delete_internal(
 	return (rc);
 }
 
+static			void
+ef10_filter_delete_auto(
+	__in		efx_nic_t *enp,
+	__in		uint32_t filter_id)
+{
+	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	uint32_t filter_idx = filter_id % EFX_EF10_FILTER_TBL_ROWS;
+
+	/*
+	 * AUTO_OLD flag is cleared since the auto filter that is to be removed
+	 * may not be the filter at the specified index itself, but the filter
+	 * that is overriden by it.
+	 */
+	ef10_filter_set_entry_not_auto_old(table, filter_idx);
+
+	(void) ef10_filter_delete_internal(enp, filter_idx,
+	    EFX_FILTER_PRI_AUTO);
+}
+
 	__checkReturn	efx_rc_t
 ef10_filter_delete(
 	__in		efx_nic_t *enp,
@@ -906,7 +1174,7 @@ ef10_filter_delete(
 	EFSYS_UNLOCK(enp->en_eslp, state);
 	locked = B_FALSE;
 
-	rc = ef10_filter_delete_internal(enp, i);
+	rc = ef10_filter_delete_internal(enp, i, EFX_FILTER_PRI_MANUAL);
 	if (rc != 0)
 		goto fail2;
 
@@ -1131,7 +1399,7 @@ ef10_filter_insert_unicast(
 	if (rc != 0)
 		goto fail1;
 
-	rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
+	rc = ef10_filter_add_internal(enp, &spec, EFX_FILTER_REPLACEMENT_NEVER,
 	    &eftp->eft_unicst_filter_indexes[eftp->eft_unicst_filter_count]);
 	if (rc != 0)
 		goto fail2;
@@ -1165,7 +1433,7 @@ ef10_filter_insert_all_unicast(
 	rc = efx_filter_spec_set_uc_def(&spec);
 	if (rc != 0)
 		goto fail1;
-	rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
+	rc = ef10_filter_add_internal(enp, &spec, EFX_FILTER_REPLACEMENT_NEVER,
 	    &eftp->eft_unicst_filter_indexes[eftp->eft_unicst_filter_count]);
 	if (rc != 0)
 		goto fail2;
@@ -1235,8 +1503,8 @@ ef10_filter_insert_multicast_list(
 			}
 		}
 
-		rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
-					    &filter_index);
+		rc = ef10_filter_add_internal(enp, &spec,
+		    EFX_FILTER_REPLACEMENT_NEVER, &filter_index);
 
 		if (rc == 0) {
 			eftp->eft_mulcst_filter_indexes[filter_count] =
@@ -1263,8 +1531,8 @@ ef10_filter_insert_multicast_list(
 			goto rollback;
 		}
 
-		rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
-					    &filter_index);
+		rc = ef10_filter_add_internal(enp, &spec,
+		    EFX_FILTER_REPLACEMENT_NEVER, &filter_index);
 
 		if (rc == 0) {
 			eftp->eft_mulcst_filter_indexes[filter_count] =
@@ -1285,7 +1553,7 @@ ef10_filter_insert_multicast_list(
 	/* Remove any filters we have inserted */
 	i = filter_count;
 	while (i--) {
-		(void) ef10_filter_delete_internal(enp,
+		ef10_filter_delete_auto(enp,
 		    eftp->eft_mulcst_filter_indexes[i]);
 	}
 	eftp->eft_mulcst_filter_count = 0;
@@ -1313,7 +1581,7 @@ ef10_filter_insert_all_multicast(
 	if (rc != 0)
 		goto fail1;
 
-	rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
+	rc = ef10_filter_add_internal(enp, &spec, EFX_FILTER_REPLACEMENT_NEVER,
 	    &eftp->eft_mulcst_filter_indexes[0]);
 	if (rc != 0)
 		goto fail2;
@@ -1416,8 +1684,9 @@ ef10_filter_insert_encap_filters(
 		if (rc != 0)
 			goto fail1;
 
-		rc = ef10_filter_add_internal(enp, &spec, B_TRUE,
-			    &table->eft_encap_filter_indexes[
+		rc = ef10_filter_add_internal(enp, &spec,
+		    EFX_FILTER_REPLACEMENT_NEVER,
+		    &table->eft_encap_filter_indexes[
 				    table->eft_encap_filter_count]);
 		if (rc != 0) {
 			if (rc != EACCES)
@@ -1446,7 +1715,7 @@ ef10_filter_remove_old(
 
 	for (i = 0; i < EFX_ARRAY_SIZE(table->eft_entry); i++) {
 		if (ef10_filter_entry_is_auto_old(table, i)) {
-			(void) ef10_filter_delete_internal(enp, i);
+			ef10_filter_delete_auto(enp, i);
 		}
 	}
 }
@@ -1521,19 +1790,19 @@ ef10_filter_reconfigure(
 		 * has rebooted, which removes hardware filters).
 		 */
 		for (i = 0; i < table->eft_unicst_filter_count; i++) {
-			(void) ef10_filter_delete_internal(enp,
+			ef10_filter_delete_auto(enp,
 					table->eft_unicst_filter_indexes[i]);
 		}
 		table->eft_unicst_filter_count = 0;
 
 		for (i = 0; i < table->eft_mulcst_filter_count; i++) {
-			(void) ef10_filter_delete_internal(enp,
+			ef10_filter_delete_auto(enp,
 					table->eft_mulcst_filter_indexes[i]);
 		}
 		table->eft_mulcst_filter_count = 0;
 
 		for (i = 0; i < table->eft_encap_filter_count; i++) {
-			(void) ef10_filter_delete_internal(enp,
+			ef10_filter_delete_auto(enp,
 					table->eft_encap_filter_indexes[i]);
 		}
 		table->eft_encap_filter_count = 0;
diff --git a/drivers/net/sfc/base/ef10_impl.h b/drivers/net/sfc/base/ef10_impl.h
index 7a0004782..67abf3b85 100644
--- a/drivers/net/sfc/base/ef10_impl.h
+++ b/drivers/net/sfc/base/ef10_impl.h
@@ -1079,6 +1079,8 @@ ef10_rx_fini(
 
 #if EFSYS_OPT_FILTER
 
+enum efx_filter_replacement_policy_e;
+
 typedef struct ef10_filter_handle_s {
 	uint32_t	efh_lo;
 	uint32_t	efh_hi;
@@ -1148,7 +1150,7 @@ ef10_filter_restore(
 ef10_filter_add(
 	__in		efx_nic_t *enp,
 	__inout		efx_filter_spec_t *spec,
-	__in		boolean_t may_replace);
+	__in		enum efx_filter_replacement_policy_e policy);
 
 	__checkReturn	efx_rc_t
 ef10_filter_delete(
diff --git a/drivers/net/sfc/base/efx.h b/drivers/net/sfc/base/efx.h
index c609c700f..d94d3c02f 100644
--- a/drivers/net/sfc/base/efx.h
+++ b/drivers/net/sfc/base/efx.h
@@ -2949,6 +2949,7 @@ typedef uint8_t efx_filter_flags_t;
 
 typedef uint32_t efx_filter_match_flags_t;
 
+/* Filter priority from lowest to highest */
 typedef enum efx_filter_priority_s {
 	EFX_FILTER_PRI_AUTO = 0,	/* Automatic filter based on device
 					 * address list or hardware
@@ -2956,6 +2957,7 @@ typedef enum efx_filter_priority_s {
 					 * by the filter implementation for
 					 * each NIC type. */
 	EFX_FILTER_PRI_MANUAL,		/* Manually configured filter */
+	EFX_FILTER_NPRI,
 } efx_filter_priority_t;
 
 /*
@@ -2970,6 +2972,11 @@ typedef struct efx_filter_spec_s {
 	uint16_t			efs_dmaq_id;
 	uint32_t			efs_rss_context;
 	uint32_t			efs_mark;
+	/*
+	 * Saved lower-priority filter. If it is set, it is restored on
+	 * filter delete operation.
+	 */
+	struct efx_filter_spec_s	*efs_overridden_spec;
 	/* Fields below here are hashed for software filter lookup */
 	uint16_t			efs_outer_vid;
 	uint16_t			efs_inner_vid;
diff --git a/drivers/net/sfc/base/efx_filter.c b/drivers/net/sfc/base/efx_filter.c
index 36332a280..9949d05bb 100644
--- a/drivers/net/sfc/base/efx_filter.c
+++ b/drivers/net/sfc/base/efx_filter.c
@@ -28,7 +28,7 @@ static	__checkReturn	efx_rc_t
 siena_filter_add(
 	__in		efx_nic_t *enp,
 	__inout		efx_filter_spec_t *spec,
-	__in		boolean_t may_replace);
+	__in		efx_filter_replacement_policy_t policy);
 
 static	__checkReturn	efx_rc_t
 siena_filter_delete(
@@ -98,7 +98,8 @@ efx_filter_insert(
 		goto fail3;
 	}
 
-	return (efop->efo_add(enp, spec, B_FALSE));
+	return (efop->efo_add(enp, spec,
+	    EFX_FILTER_REPLACEMENT_HIGHER_PRIORITY));
 
 fail3:
 	EFSYS_PROBE(fail3);
@@ -1444,7 +1445,7 @@ static	 __checkReturn	efx_rc_t
 siena_filter_add(
 	__in		efx_nic_t *enp,
 	__inout		efx_filter_spec_t *spec,
-	__in		boolean_t may_replace)
+	__in		efx_filter_replacement_policy_t policy)
 {
 	efx_rc_t rc;
 	siena_filter_spec_t sf_spec;
@@ -1485,9 +1486,17 @@ siena_filter_add(
 	saved_sf_spec = &sftp->sft_spec[filter_idx];
 
 	if (siena_filter_test_used(sftp, filter_idx)) {
-		if (may_replace == B_FALSE) {
+		/* All Siena filter are considered the same priority */
+		switch (policy) {
+		case EFX_FILTER_REPLACEMENT_NEVER:
+		case EFX_FILTER_REPLACEMENT_HIGHER_PRIORITY:
 			rc = EEXIST;
 			goto fail4;
+		case EFX_FILTER_REPLACEMENT_HIGHER_OR_EQUAL_PRIORITY:
+			break;
+		default:
+			EFSYS_ASSERT(0);
+			break;
 		}
 	}
 	siena_filter_set_used(sftp, filter_idx);
diff --git a/drivers/net/sfc/base/efx_impl.h b/drivers/net/sfc/base/efx_impl.h
index 85d984f65..9755f4dfd 100644
--- a/drivers/net/sfc/base/efx_impl.h
+++ b/drivers/net/sfc/base/efx_impl.h
@@ -246,12 +246,31 @@ typedef struct efx_phy_ops_s {
 } efx_phy_ops_t;
 
 #if EFSYS_OPT_FILTER
+
+/*
+ * Policy for replacing existing filter when inserting a new one.
+ * Note that all policies allow for storing the new lower priority
+ * filters as overridden by existing higher priority ones. It is needed
+ * to restore the lower priority filters on higher priority ones removal.
+ */
+typedef enum efx_filter_replacement_policy_e {
+	/* Cannot replace existing filter */
+	EFX_FILTER_REPLACEMENT_NEVER,
+	/* Higher priority filters can replace lower priotiry ones */
+	EFX_FILTER_REPLACEMENT_HIGHER_PRIORITY,
+	/*
+	 * Higher priority filters can replace lower priority ones and
+	 * equal priority filters can replace each other.
+	 */
+	EFX_FILTER_REPLACEMENT_HIGHER_OR_EQUAL_PRIORITY,
+} efx_filter_replacement_policy_t;
+
 typedef struct efx_filter_ops_s {
 	efx_rc_t	(*efo_init)(efx_nic_t *);
 	void		(*efo_fini)(efx_nic_t *);
 	efx_rc_t	(*efo_restore)(efx_nic_t *);
 	efx_rc_t	(*efo_add)(efx_nic_t *, efx_filter_spec_t *,
-				   boolean_t may_replace);
+				   efx_filter_replacement_policy_t policy);
 	efx_rc_t	(*efo_delete)(efx_nic_t *, efx_filter_spec_t *);
 	efx_rc_t	(*efo_supported_filters)(efx_nic_t *, uint32_t *,
 				   size_t, size_t *);
-- 
2.17.1


  parent reply	other threads:[~2020-03-10  9:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10  9:48 [dpdk-dev] [PATCH 0/5] net/sfc: fix manual and auto filters clash handling Andrew Rybchenko
2020-03-10  9:48 ` [dpdk-dev] [PATCH 1/5] net/sfc: set priority of created filters to manual Andrew Rybchenko
2020-03-10  9:48 ` [dpdk-dev] [PATCH 2/5] net/sfc/base: reduce filter priorities to implemented only Andrew Rybchenko
2020-03-10  9:48 ` [dpdk-dev] [PATCH 3/5] net/sfc/base: reject automatic filter creation by users Andrew Rybchenko
2020-03-10  9:48 ` [dpdk-dev] [PATCH 4/5] net/sfc/base: refactor filter lookup loop in EF10 Andrew Rybchenko
2020-03-10  9:48 ` Andrew Rybchenko [this message]
2020-03-11 11:30 ` [dpdk-dev] [PATCH 0/5] net/sfc: fix manual and auto filters clash handling Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1583833719-6597-6-git-send-email-arybchenko@solarflare.com \
    --to=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=igor.romanov@oktetlabs.ru \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).