[dpdk-dev] [PATCH 0/6] net/sfc: improve Rx mode handling on VF

[dpdk-dev] [PATCH 0/6] net/sfc: improve Rx mode handling on VF

[Andrew Rybchenko]

VFs (as well as unpriviledged PFs) are not always allowed to
enable promiscuous and all-multicast. Handle it properly and
ensure consistency for settings applied and reported.

Base driver patches have warnings/errors from checkpatch.sh
since coding style differs a bit.

Return of non-negative errno is OK as well, since driver does
positive to negative errno conversion on return from callbacks.

Igor Romanov (6):
  net/sfc/base: refactor filters cleanup in reconfigure
  net/sfc/base: refactor filters mark in reconfigure
  net/sfc/base: refactor unicast filters reconfiguration
  net/sfc/base: refactor mcast filters reconfiguration
  net/sfc/base: add API to get currently operating filters
  net/sfc: check actual all multicast unknown unicast filters

 drivers/net/sfc/base/ef10_filter.c | 314 +++++++++++++++++++----------
 drivers/net/sfc/base/efx.h         |   6 +
 drivers/net/sfc/base/efx_impl.h    |   2 +
 drivers/net/sfc/base/efx_mac.c     |  15 ++
 drivers/net/sfc/base/siena_mac.c   |   2 +
 drivers/net/sfc/sfc.h              |   1 +
 drivers/net/sfc/sfc_ethdev.c       |  16 +-
 drivers/net/sfc/sfc_port.c         |  62 +++++-
 drivers/net/sfc/sfc_rx.c           |   4 +-
 9 files changed, 310 insertions(+), 112 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/6] net/sfc/base: refactor filters cleanup in reconfigure

[Andrew Rybchenko]

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

Refactor the filters cleanup stage of the reconfigure function
to make it clearer and allow for more convenient further changes.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 44 ++++++++++++++++++------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 12802a3d13..74d06ecf57 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -1756,6 +1756,31 @@ ef10_filter_get_workarounds(
 
 }
 
+static			void
+ef10_filter_remove_all_existing_filters(
+	__in				efx_nic_t *enp)
+{
+	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	unsigned int i;
+
+	for (i = 0; i < table->eft_unicst_filter_count; i++) {
+		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++) {
+		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++) {
+		ef10_filter_delete_auto(enp,
+				table->eft_encap_filter_indexes[i]);
+	}
+	table->eft_encap_filter_count = 0;
+}
 
 /*
  * Reconfigure all filters.
@@ -1789,24 +1814,7 @@ ef10_filter_reconfigure(
 		 * filters must be removed (ignore errors in case the MC
 		 * has rebooted, which removes hardware filters).
 		 */
-		for (i = 0; i < table->eft_unicst_filter_count; i++) {
-			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++) {
-			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++) {
-			ef10_filter_delete_auto(enp,
-					table->eft_encap_filter_indexes[i]);
-		}
-		table->eft_encap_filter_count = 0;
-
+		ef10_filter_remove_all_existing_filters(enp);
 		return (0);
 	}
 
-- 
2.17.1

[dpdk-dev] [PATCH 2/6] net/sfc/base: refactor filters mark in reconfigure

[Andrew Rybchenko]

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

Refactor the filters mark stage of the reconfigure function
to make it clearer and allow for more convenient further changes.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 34 +++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 74d06ecf57..5033ff75ac 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -1782,6 +1782,27 @@ ef10_filter_remove_all_existing_filters(
 	table->eft_encap_filter_count = 0;
 }
 
+static			void
+ef10_filter_mark_old_filters(
+	__in				efx_nic_t *enp)
+{
+	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	unsigned int i;
+
+	for (i = 0; i < table->eft_unicst_filter_count; i++) {
+		ef10_filter_set_entry_auto_old(table,
+					table->eft_unicst_filter_indexes[i]);
+	}
+	for (i = 0; i < table->eft_mulcst_filter_count; i++) {
+		ef10_filter_set_entry_auto_old(table,
+					table->eft_mulcst_filter_indexes[i]);
+	}
+	for (i = 0; i < table->eft_encap_filter_count; i++) {
+		ef10_filter_set_entry_auto_old(table,
+					table->eft_encap_filter_indexes[i]);
+	}
+}
+
 /*
  * Reconfigure all filters.
  * If all_unicst and/or all mulcst filters cannot be applied then
@@ -1824,18 +1845,7 @@ ef10_filter_reconfigure(
 		filter_flags = 0;
 
 	/* Mark old filters which may need to be removed */
-	for (i = 0; i < table->eft_unicst_filter_count; i++) {
-		ef10_filter_set_entry_auto_old(table,
-					table->eft_unicst_filter_indexes[i]);
-	}
-	for (i = 0; i < table->eft_mulcst_filter_count; i++) {
-		ef10_filter_set_entry_auto_old(table,
-					table->eft_mulcst_filter_indexes[i]);
-	}
-	for (i = 0; i < table->eft_encap_filter_count; i++) {
-		ef10_filter_set_entry_auto_old(table,
-					table->eft_encap_filter_indexes[i]);
-	}
+	ef10_filter_mark_old_filters(enp);
 
 	/*
 	 * Insert or renew unicast filters.
-- 
2.17.1

[dpdk-dev] [PATCH 3/6] net/sfc/base: refactor unicast filters reconfiguration

[Andrew Rybchenko]

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

Refactor the unicast filter reconfiguration stage of the reconfigure
function to make it clearer and allow for more convenient further
changes.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 83 ++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 27 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 5033ff75ac..35d675a65d 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -1803,6 +1803,54 @@ ef10_filter_mark_old_filters(
 	}
 }
 
+static	__checkReturn	efx_rc_t
+ef10_filter_insert_renew_unicst_filters(
+	__in				efx_nic_t *enp,
+	__in_ecount(6)			uint8_t const *mac_addr,
+	__in				boolean_t all_unicst,
+	__in				efx_filter_flags_t filter_flags,
+	__out				boolean_t *all_unicst_inserted)
+{
+	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	efx_rc_t rc;
+
+	/*
+	 * Firmware does not perform chaining on unicast filters. As traffic is
+	 * therefore only delivered to the first matching filter, we should
+	 * always insert the specific filter for our MAC address, to try and
+	 * ensure we get that traffic.
+	 *
+	 * (If the filter for our MAC address has already been inserted by
+	 * another function, we won't receive traffic sent to us, even if we
+	 * insert a unicast mismatch filter. To prevent traffic stealing, this
+	 * therefore relies on the privilege model only allowing functions to
+	 * insert filters for their own MAC address unless explicitly given
+	 * additional privileges by the user. This also means that, even on a
+	 * priviliged function, inserting a unicast mismatch filter may not
+	 * catch all traffic in multi PCI function scenarios.)
+	 */
+	table->eft_unicst_filter_count = 0;
+	rc = ef10_filter_insert_unicast(enp, mac_addr, filter_flags);
+	*all_unicst_inserted = B_FALSE;
+	if (all_unicst || (rc != 0)) {
+		efx_rc_t all_unicst_rc;
+
+		all_unicst_rc = ef10_filter_insert_all_unicast(enp,
+						    filter_flags);
+		if (all_unicst_rc == 0) {
+			*all_unicst_inserted = B_TRUE;
+		} else if (rc != 0)
+			goto fail1;
+	}
+
+	return (0);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 /*
  * Reconfigure all filters.
  * If all_unicst and/or all mulcst filters cannot be applied then
@@ -1824,7 +1872,7 @@ ef10_filter_reconfigure(
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
 	efx_filter_flags_t filter_flags;
 	unsigned int i;
-	efx_rc_t all_unicst_rc = 0;
+	boolean_t all_unicst_inserted = B_FALSE;
 	efx_rc_t all_mulcst_rc = 0;
 	efx_rc_t rc;
 
@@ -1847,31 +1895,12 @@ ef10_filter_reconfigure(
 	/* Mark old filters which may need to be removed */
 	ef10_filter_mark_old_filters(enp);
 
-	/*
-	 * Insert or renew unicast filters.
-	 *
-	 * Firmware does not perform chaining on unicast filters. As traffic is
-	 * therefore only delivered to the first matching filter, we should
-	 * always insert the specific filter for our MAC address, to try and
-	 * ensure we get that traffic.
-	 *
-	 * (If the filter for our MAC address has already been inserted by
-	 * another function, we won't receive traffic sent to us, even if we
-	 * insert a unicast mismatch filter. To prevent traffic stealing, this
-	 * therefore relies on the privilege model only allowing functions to
-	 * insert filters for their own MAC address unless explicitly given
-	 * additional privileges by the user. This also means that, even on a
-	 * priviliged function, inserting a unicast mismatch filter may not
-	 * catch all traffic in multi PCI function scenarios.)
-	 */
-	table->eft_unicst_filter_count = 0;
-	rc = ef10_filter_insert_unicast(enp, mac_addr, filter_flags);
-	if (all_unicst || (rc != 0)) {
-		all_unicst_rc = ef10_filter_insert_all_unicast(enp,
-						    filter_flags);
-		if ((rc != 0) && (all_unicst_rc != 0))
-			goto fail1;
-	}
+	/* Insert or renew unicast filters */
+	rc = ef10_filter_insert_renew_unicst_filters(enp, mac_addr, all_unicst,
+						     filter_flags,
+						     &all_unicst_inserted);
+	if (rc != 0)
+		goto fail1;
 
 	/*
 	 * WORKAROUND_BUG26807 controls firmware support for chained multicast
@@ -1974,7 +2003,7 @@ ef10_filter_reconfigure(
 	ef10_filter_remove_old(enp);
 
 	/* report if any optional flags were rejected */
-	if (((all_unicst != B_FALSE) && (all_unicst_rc != 0)) ||
+	if (((all_unicst != B_FALSE) && (all_unicst_inserted == B_FALSE)) ||
 	    ((all_mulcst != B_FALSE) && (all_mulcst_rc != 0))) {
 		rc = ENOTSUP;
 	}
-- 
2.17.1

[dpdk-dev] [PATCH 4/6] net/sfc/base: refactor mcast filters reconfiguration

[Andrew Rybchenko]

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

Refactor the multicast filter reconfiguration stage of the reconfigure
function to make it clearer and allow for more convenient further
changes.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 143 +++++++++++++++++++----------
 1 file changed, 92 insertions(+), 51 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 35d675a65d..0a8744c09e 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -1851,6 +1851,89 @@ ef10_filter_insert_renew_unicst_filters(
 	return (rc);
 }
 
+static	__checkReturn	efx_rc_t
+ef10_filter_insert_renew_mulcst_filters(
+	__in				efx_nic_t *enp,
+	__in				boolean_t mulcst,
+	__in				boolean_t all_mulcst,
+	__in				boolean_t brdcst,
+	__in_ecount(6*count)		uint8_t const *addrs,
+	__in				uint32_t count,
+	__in				efx_filter_flags_t filter_flags,
+	__in				boolean_t all_unicst_inserted,
+	__out				boolean_t *all_mulcst_inserted)
+{
+	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	efx_nic_cfg_t *encp = &enp->en_nic_cfg;
+	efx_rc_t rc;
+
+	*all_mulcst_inserted = B_FALSE;
+
+	if (all_mulcst == B_TRUE) {
+		efx_rc_t all_mulcst_rc;
+
+		/*
+		 * Insert the all multicast filter. If that fails, try to insert
+		 * all of our multicast filters (but without rollback on
+		 * failure).
+		 */
+		all_mulcst_rc = ef10_filter_insert_all_multicast(enp,
+							    filter_flags);
+		if (all_mulcst_rc == 0) {
+			*all_mulcst_inserted = B_TRUE;
+		} else {
+			rc = ef10_filter_insert_multicast_list(enp, B_TRUE,
+			    brdcst, addrs, count, filter_flags, B_FALSE);
+			if (rc != 0)
+				goto fail1;
+		}
+	} else {
+		/*
+		 * Insert filters for multicast addresses.
+		 * If any insertion fails, then rollback and try to insert the
+		 * all multicast filter instead.
+		 * If that also fails, try to insert all of the multicast
+		 * filters (but without rollback on failure).
+		 */
+		rc = ef10_filter_insert_multicast_list(enp, mulcst, brdcst,
+			    addrs, count, filter_flags, B_TRUE);
+		if (rc != 0) {
+			if ((table->eft_using_all_mulcst == B_FALSE) &&
+			    (encp->enc_bug26807_workaround == B_TRUE)) {
+				/*
+				 * Multicast filter chaining is on, so remove
+				 * old filters before inserting the multicast
+				 * all filter to avoid duplicate delivery caused
+				 * by packets matching multiple filters.
+				 */
+				ef10_filter_remove_old(enp);
+			}
+
+			rc = ef10_filter_insert_all_multicast(enp,
+							    filter_flags);
+			if (rc == 0) {
+				*all_mulcst_inserted = B_TRUE;
+			} else {
+				rc = ef10_filter_insert_multicast_list(enp,
+				    mulcst, brdcst,
+				    addrs, count, filter_flags, B_FALSE);
+				if (rc != 0)
+					goto fail2;
+			}
+		}
+	}
+
+	return (0);
+
+fail2:
+	EFSYS_PROBE1(fail2, efx_rc_t, rc);
+
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+
+	return (rc);
+}
+
 /*
  * Reconfigure all filters.
  * If all_unicst and/or all mulcst filters cannot be applied then
@@ -1873,7 +1956,7 @@ ef10_filter_reconfigure(
 	efx_filter_flags_t filter_flags;
 	unsigned int i;
 	boolean_t all_unicst_inserted = B_FALSE;
-	efx_rc_t all_mulcst_rc = 0;
+	boolean_t all_mulcst_inserted = B_FALSE;
 	efx_rc_t rc;
 
 	if (table->eft_default_rxq == NULL) {
@@ -1944,53 +2027,13 @@ ef10_filter_reconfigure(
 	}
 
 	/* Insert or renew multicast filters */
-	if (all_mulcst == B_TRUE) {
-		/*
-		 * Insert the all multicast filter. If that fails, try to insert
-		 * all of our multicast filters (but without rollback on
-		 * failure).
-		 */
-		all_mulcst_rc = ef10_filter_insert_all_multicast(enp,
-							    filter_flags);
-		if (all_mulcst_rc != 0) {
-			rc = ef10_filter_insert_multicast_list(enp, B_TRUE,
-			    brdcst, addrs, count, filter_flags, B_FALSE);
-			if (rc != 0)
-				goto fail3;
-		}
-	} else {
-		/*
-		 * Insert filters for multicast addresses.
-		 * If any insertion fails, then rollback and try to insert the
-		 * all multicast filter instead.
-		 * If that also fails, try to insert all of the multicast
-		 * filters (but without rollback on failure).
-		 */
-		rc = ef10_filter_insert_multicast_list(enp, mulcst, brdcst,
-			    addrs, count, filter_flags, B_TRUE);
-		if (rc != 0) {
-			if ((table->eft_using_all_mulcst == B_FALSE) &&
-			    (encp->enc_bug26807_workaround == B_TRUE)) {
-				/*
-				 * Multicast filter chaining is on, so remove
-				 * old filters before inserting the multicast
-				 * all filter to avoid duplicate delivery caused
-				 * by packets matching multiple filters.
-				 */
-				ef10_filter_remove_old(enp);
-			}
-
-			rc = ef10_filter_insert_all_multicast(enp,
-							    filter_flags);
-			if (rc != 0) {
-				rc = ef10_filter_insert_multicast_list(enp,
-				    mulcst, brdcst,
-				    addrs, count, filter_flags, B_FALSE);
-				if (rc != 0)
-					goto fail4;
-			}
-		}
-	}
+	rc = ef10_filter_insert_renew_mulcst_filters(enp, mulcst, all_mulcst,
+						     brdcst, addrs, count,
+						     filter_flags,
+						     all_unicst_inserted,
+						     &all_mulcst_inserted);
+	if (rc != 0)
+		goto fail3;
 
 	if (encp->enc_tunnel_encapsulations_supported != 0) {
 		/* Try to insert filters for encapsulated packets. */
@@ -2004,14 +2047,12 @@ ef10_filter_reconfigure(
 
 	/* report if any optional flags were rejected */
 	if (((all_unicst != B_FALSE) && (all_unicst_inserted == B_FALSE)) ||
-	    ((all_mulcst != B_FALSE) && (all_mulcst_rc != 0))) {
+	    ((all_mulcst != B_FALSE) && (all_mulcst_inserted == B_FALSE))) {
 		rc = ENOTSUP;
 	}
 
 	return (rc);
 
-fail4:
-	EFSYS_PROBE(fail4);
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
-- 
2.17.1

[dpdk-dev] [PATCH 5/6] net/sfc/base: add API to get currently operating filters

[Andrew Rybchenko]

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

Unknown unicast filter creation may fail because of insufficient
permissions on VF. This failure is handled internally in libefx MAC
reconfiguration without any way for a user to know if it happened.
Making the MAC reconfiguration forward error code of filter
reconfiguration would be too destructive to the existing code
that may rely on the function never returning that error.

Add an API for getting the status of current unknown unicast and
all multicast filters since user must know that requested
filters are actually applied.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/base/ef10_filter.c | 22 ++++++++++++++++++++++
 drivers/net/sfc/base/efx.h         |  6 ++++++
 drivers/net/sfc/base/efx_impl.h    |  2 ++
 drivers/net/sfc/base/efx_mac.c     | 15 +++++++++++++++
 drivers/net/sfc/base/siena_mac.c   |  2 ++
 5 files changed, 47 insertions(+)

diff --git a/drivers/net/sfc/base/ef10_filter.c b/drivers/net/sfc/base/ef10_filter.c
index 0a8744c09e..0dff8397af 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -1761,6 +1761,7 @@ ef10_filter_remove_all_existing_filters(
 	__in				efx_nic_t *enp)
 {
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	efx_port_t *epp = &(enp->en_port);
 	unsigned int i;
 
 	for (i = 0; i < table->eft_unicst_filter_count; i++) {
@@ -1780,6 +1781,9 @@ ef10_filter_remove_all_existing_filters(
 				table->eft_encap_filter_indexes[i]);
 	}
 	table->eft_encap_filter_count = 0;
+
+	epp->ep_all_unicst_inserted = B_FALSE;
+	epp->ep_all_mulcst_inserted = B_FALSE;
 }
 
 static			void
@@ -1812,6 +1816,7 @@ ef10_filter_insert_renew_unicst_filters(
 	__out				boolean_t *all_unicst_inserted)
 {
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
+	efx_port_t *epp = &(enp->en_port);
 	efx_rc_t rc;
 
 	/*
@@ -1839,6 +1844,7 @@ ef10_filter_insert_renew_unicst_filters(
 						    filter_flags);
 		if (all_unicst_rc == 0) {
 			*all_unicst_inserted = B_TRUE;
+			epp->ep_all_unicst_inserted = B_TRUE;
 		} else if (rc != 0)
 			goto fail1;
 	}
@@ -1865,6 +1871,7 @@ ef10_filter_insert_renew_mulcst_filters(
 {
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
 	efx_nic_cfg_t *encp = &enp->en_nic_cfg;
+	efx_port_t *epp = &(enp->en_port);
 	efx_rc_t rc;
 
 	*all_mulcst_inserted = B_FALSE;
@@ -1880,6 +1887,7 @@ ef10_filter_insert_renew_mulcst_filters(
 		all_mulcst_rc = ef10_filter_insert_all_multicast(enp,
 							    filter_flags);
 		if (all_mulcst_rc == 0) {
+			epp->ep_all_mulcst_inserted = B_TRUE;
 			*all_mulcst_inserted = B_TRUE;
 		} else {
 			rc = ef10_filter_insert_multicast_list(enp, B_TRUE,
@@ -1907,11 +1915,16 @@ ef10_filter_insert_renew_mulcst_filters(
 				 * by packets matching multiple filters.
 				 */
 				ef10_filter_remove_old(enp);
+				if (all_unicst_inserted == B_FALSE)
+					epp->ep_all_unicst_inserted = B_FALSE;
+				if (*all_mulcst_inserted == B_FALSE)
+					epp->ep_all_mulcst_inserted = B_FALSE;
 			}
 
 			rc = ef10_filter_insert_all_multicast(enp,
 							    filter_flags);
 			if (rc == 0) {
+				epp->ep_all_mulcst_inserted = B_TRUE;
 				*all_mulcst_inserted = B_TRUE;
 			} else {
 				rc = ef10_filter_insert_multicast_list(enp,
@@ -1952,6 +1965,7 @@ ef10_filter_reconfigure(
 	__in				uint32_t count)
 {
 	efx_nic_cfg_t *encp = &enp->en_nic_cfg;
+	efx_port_t *epp = &(enp->en_port);
 	ef10_filter_table_t *table = enp->en_filter.ef_ef10_filter_table;
 	efx_filter_flags_t filter_flags;
 	unsigned int i;
@@ -2024,6 +2038,10 @@ ef10_filter_reconfigure(
 		 * multicast filters.
 		 */
 		ef10_filter_remove_old(enp);
+		if (all_unicst_inserted == B_FALSE)
+			epp->ep_all_unicst_inserted = B_FALSE;
+
+		epp->ep_all_mulcst_inserted = B_FALSE;
 	}
 
 	/* Insert or renew multicast filters */
@@ -2044,6 +2062,10 @@ ef10_filter_reconfigure(
 
 	/* Remove old filters which were not renewed */
 	ef10_filter_remove_old(enp);
+	if (all_unicst_inserted == B_FALSE)
+		epp->ep_all_unicst_inserted = B_FALSE;
+	if (all_mulcst_inserted == B_FALSE)
+		epp->ep_all_mulcst_inserted = B_FALSE;
 
 	/* report if any optional flags were rejected */
 	if (((all_unicst != B_FALSE) && (all_unicst_inserted == B_FALSE)) ||
diff --git a/drivers/net/sfc/base/efx.h b/drivers/net/sfc/base/efx.h
index d94d3c02f7..86733f9317 100644
--- a/drivers/net/sfc/base/efx.h
+++ b/drivers/net/sfc/base/efx.h
@@ -544,6 +544,12 @@ efx_mac_filter_set(
 	__in				boolean_t all_mulcst,
 	__in				boolean_t brdcst);
 
+extern					void
+efx_mac_filter_get_all_ucast_mcast(
+	__in				efx_nic_t *enp,
+	__out				boolean_t *all_unicst,
+	__out				boolean_t *all_mulcst);
+
 extern	__checkReturn	efx_rc_t
 efx_mac_multicast_list_set(
 	__in				efx_nic_t *enp,
diff --git a/drivers/net/sfc/base/efx_impl.h b/drivers/net/sfc/base/efx_impl.h
index 9755f4dfd2..adad7900f6 100644
--- a/drivers/net/sfc/base/efx_impl.h
+++ b/drivers/net/sfc/base/efx_impl.h
@@ -307,8 +307,10 @@ typedef struct efx_port_s {
 	uint8_t			ep_mac_addr[6];
 	efx_link_mode_t		ep_link_mode;
 	boolean_t		ep_all_unicst;
+	boolean_t		ep_all_unicst_inserted;
 	boolean_t		ep_mulcst;
 	boolean_t		ep_all_mulcst;
+	boolean_t		ep_all_mulcst_inserted;
 	boolean_t		ep_brdcst;
 	unsigned int		ep_fcntl;
 	boolean_t		ep_fcntl_autoneg;
diff --git a/drivers/net/sfc/base/efx_mac.c b/drivers/net/sfc/base/efx_mac.c
index 673bc4f4ee..b13321065a 100644
--- a/drivers/net/sfc/base/efx_mac.c
+++ b/drivers/net/sfc/base/efx_mac.c
@@ -218,6 +218,21 @@ efx_mac_filter_set(
 	return (rc);
 }
 
+					void
+efx_mac_filter_get_all_ucast_mcast(
+	__in				efx_nic_t *enp,
+	__out				boolean_t *all_unicst,
+	__out				boolean_t *all_mulcst)
+{
+	efx_port_t *epp = &(enp->en_port);
+
+	EFSYS_ASSERT3U(enp->en_magic, ==, EFX_NIC_MAGIC);
+	EFSYS_ASSERT3U(enp->en_mod_flags, &, EFX_MOD_PORT);
+
+	*all_unicst = epp->ep_all_unicst_inserted;
+	*all_mulcst = epp->ep_all_mulcst_inserted;
+}
+
 	__checkReturn			efx_rc_t
 efx_mac_drain(
 	__in				efx_nic_t *enp,
diff --git a/drivers/net/sfc/base/siena_mac.c b/drivers/net/sfc/base/siena_mac.c
index 928dfc340a..157d54c38f 100644
--- a/drivers/net/sfc/base/siena_mac.c
+++ b/drivers/net/sfc/base/siena_mac.c
@@ -107,6 +107,7 @@ siena_mac_reconfigure(
 		rc = req.emr_rc;
 		goto fail1;
 	}
+	epp->ep_all_unicst_inserted = epp->ep_all_unicst;
 
 	/* Push multicast hash */
 
@@ -158,6 +159,7 @@ siena_mac_reconfigure(
 		rc = req.emr_rc;
 		goto fail2;
 	}
+	epp->ep_all_mulcst_inserted = epp->ep_all_mulcst;
 
 	return (0);
 
-- 
2.17.1

[dpdk-dev] [PATCH 6/6] net/sfc: check actual all multicast unknown unicast filters

[Andrew Rybchenko]

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

Check that unknown unicast and unknown multicast filters are
applied and return an error if they are not applied. The error
is used in promiscuous and all multicast mode enable and disable
callbacks.

Signed-off-by: Igor Romanov <igor.romanov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc.h        |  1 +
 drivers/net/sfc/sfc_ethdev.c | 16 +++++++---
 drivers/net/sfc/sfc_port.c   | 62 +++++++++++++++++++++++++++++++++---
 drivers/net/sfc/sfc_rx.c     |  4 +--
 4 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 2520cf2b7c..ceb23402a5 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -407,6 +407,7 @@ void sfc_port_link_mode_to_info(efx_link_mode_t link_mode,
 int sfc_port_update_mac_stats(struct sfc_adapter *sa);
 int sfc_port_reset_mac_stats(struct sfc_adapter *sa);
 int sfc_set_rx_mode(struct sfc_adapter *sa);
+int sfc_set_rx_mode_unchecked(struct sfc_adapter *sa);
 
 
 #ifdef __cplusplus
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f8867b0ec0..de490eb90d 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -393,8 +393,16 @@ sfc_dev_filter_set(struct rte_eth_dev *dev, enum sfc_dev_filter_mode mode,
 		} else if ((sa->state == SFC_ADAPTER_STARTED) &&
 			   ((rc = sfc_set_rx_mode(sa)) != 0)) {
 			*toggle = !(enabled);
-			sfc_warn(sa, "Failed to %s %s mode",
-				 ((enabled) ? "enable" : "disable"), desc);
+			sfc_warn(sa, "Failed to %s %s mode, rc = %d",
+				 ((enabled) ? "enable" : "disable"), desc, rc);
+
+			/*
+			 * For promiscuous and all-multicast filters a
+			 * permission failure should be reported as an
+			 * unsupported filter.
+			 */
+			if (rc == EPERM)
+				rc = ENOTSUP;
 		}
 	}
 
@@ -1060,12 +1068,12 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 		 * has no effect on received traffic, therefore
 		 * we also need to update unicast filters
 		 */
-		rc = sfc_set_rx_mode(sa);
+		rc = sfc_set_rx_mode_unchecked(sa);
 		if (rc != 0) {
 			sfc_err(sa, "cannot set filter (rc = %u)", rc);
 			/* Rollback the old address */
 			(void)efx_mac_addr_set(sa->nic, old_addr->addr_bytes);
-			(void)sfc_set_rx_mode(sa);
+			(void)sfc_set_rx_mode_unchecked(sa);
 		}
 	} else {
 		sfc_warn(sa, "cannot set MAC address with filters installed");
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 23313e125f..0ddefdefe8 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -239,7 +239,7 @@ sfc_port_start(struct sfc_adapter *sa)
 				B_TRUE : B_FALSE;
 		port->allmulti = (sa->eth_dev->data->all_multicast != 0) ?
 				 B_TRUE : B_FALSE;
-		rc = sfc_set_rx_mode(sa);
+		rc = sfc_set_rx_mode_unchecked(sa);
 		if (rc != 0)
 			goto fail_mac_filter_set;
 
@@ -485,16 +485,70 @@ sfc_port_detach(struct sfc_adapter *sa)
 	sfc_log_init(sa, "done");
 }
 
+static boolean_t
+sfc_get_requested_all_ucast(struct sfc_port *port)
+{
+	return port->promisc;
+}
+
+static boolean_t
+sfc_get_requested_all_mcast(struct sfc_port *port)
+{
+	return port->promisc || port->allmulti;
+}
+
+int
+sfc_set_rx_mode_unchecked(struct sfc_adapter *sa)
+{
+	struct sfc_port *port = &sa->port;
+	boolean_t requested_all_ucast = sfc_get_requested_all_ucast(port);
+	boolean_t requested_all_mcast = sfc_get_requested_all_mcast(port);
+	int rc;
+
+	rc = efx_mac_filter_set(sa->nic, requested_all_ucast, B_TRUE,
+				requested_all_mcast, B_TRUE);
+	if (rc != 0)
+		return rc;
+
+	return 0;
+}
+
 int
 sfc_set_rx_mode(struct sfc_adapter *sa)
 {
 	struct sfc_port *port = &sa->port;
+	boolean_t old_all_ucast;
+	boolean_t old_all_mcast;
+	boolean_t requested_all_ucast = sfc_get_requested_all_ucast(port);
+	boolean_t requested_all_mcast = sfc_get_requested_all_mcast(port);
+	boolean_t actual_all_ucast;
+	boolean_t actual_all_mcast;
 	int rc;
 
-	rc = efx_mac_filter_set(sa->nic, port->promisc, B_TRUE,
-				port->promisc || port->allmulti, B_TRUE);
+	efx_mac_filter_get_all_ucast_mcast(sa->nic, &old_all_ucast,
+					   &old_all_mcast);
 
-	return rc;
+	rc = sfc_set_rx_mode_unchecked(sa);
+	if (rc != 0)
+		return rc;
+
+	efx_mac_filter_get_all_ucast_mcast(sa->nic, &actual_all_ucast,
+					   &actual_all_mcast);
+
+	if (actual_all_ucast != requested_all_ucast ||
+	    actual_all_mcast != requested_all_mcast) {
+		/*
+		 * MAC filter set succeeded but not all requested modes
+		 * were applied. The rollback is necessary to bring back the
+		 * consistent old state.
+		 */
+		(void)efx_mac_filter_set(sa->nic, old_all_ucast, B_TRUE,
+					 old_all_mcast, B_TRUE);
+
+		return EPERM;
+	}
+
+	return 0;
 }
 
 void
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 891709fd04..183589c639 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -720,7 +720,7 @@ sfc_rx_default_rxq_set_filter(struct sfc_adapter *sa, struct sfc_rxq *rxq)
 
 		port->promisc = B_FALSE;
 		sa->eth_dev->data->promiscuous = 0;
-		rc = sfc_set_rx_mode(sa);
+		rc = sfc_set_rx_mode_unchecked(sa);
 		if (rc != 0)
 			return rc;
 
@@ -734,7 +734,7 @@ sfc_rx_default_rxq_set_filter(struct sfc_adapter *sa, struct sfc_rxq *rxq)
 
 		port->allmulti = B_FALSE;
 		sa->eth_dev->data->all_multicast = 0;
-		rc = sfc_set_rx_mode(sa);
+		rc = sfc_set_rx_mode_unchecked(sa);
 		if (rc != 0)
 			return rc;
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH 0/6] net/sfc: improve Rx mode handling on VF

[Ferruh Yigit]

On 3/30/2020 11:25 AM, Andrew Rybchenko wrote:
> VFs (as well as unpriviledged PFs) are not always allowed to
> enable promiscuous and all-multicast. Handle it properly and
> ensure consistency for settings applied and reported.
> 
> Base driver patches have warnings/errors from checkpatch.sh
> since coding style differs a bit.
> 
> Return of non-negative errno is OK as well, since driver does
> positive to negative errno conversion on return from callbacks.
> 
> Igor Romanov (6):
>   net/sfc/base: refactor filters cleanup in reconfigure
>   net/sfc/base: refactor filters mark in reconfigure
>   net/sfc/base: refactor unicast filters reconfiguration
>   net/sfc/base: refactor mcast filters reconfiguration
>   net/sfc/base: add API to get currently operating filters
>   net/sfc: check actual all multicast unknown unicast filters
> 

Series applied to dpdk-next-net/master, thanks.