All of lore.kernel.org
 help / color / mirror / Atom feed
* [next-queue PATCH v3 0/8] igb: offloading of receive filters
@ 2018-03-08  0:37 ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

Hi,

Changes from v3:
 - Addressed review comments from Aaron F. Brown and
   Jakub Kicinski;

Changes from v2:
 - Addressed review comments from Jakub Kicinski, mostly about coding
   style adjustments and more consistent error reporting;

Changes from v1:
 - Addressed review comments from Alexander Duyck and Florian
   Fainelli;
 - Adding and removing cls_flower filters are now proposed in the same
   patch;
 - cls_flower filters are kept in a separated list from "ethtool"
   filters (so that section of the original cover letter is no longer
   valid);
 - The patch adding support for ethtool filters is now independent from
   the rest of the series;

Original cover letter:

This series enables some ethtool and tc-flower filters to be offloaded
to igb-based network controllers. This is useful when the system
configurator want to steer kinds of traffic to a specific hardware
queue.

The first two commits are bug fixes.

The basis of this series is to export the internal API used to
configure address filters, so they can be used by ethtool, and
extending the functionality so an source address can be handled.

Then, we enable the tc-flower offloading implementation to re-use the
same infrastructure as ethtool, and storing them in the per-adapter
"nfc" (Network Filter Config?) list. But for consistency, for
destructive access they are separated, i.e. an filter added by
tc-flower can only be removed by tc-flower, but ethtool can read them
all.

Only support for VLAN Prio, Source and Destination MAC Address, and
Ethertype is enabled for now.

Open question:
  - igb is initialized with the number of traffic classes as 1, if we
  want to use multiple traffic classes we need to increase this value,
  the only way I could find is to use mqprio (for example). Should igb
  be initialized with, say, the number of queues as its "num_tc"?

Vinicius Costa Gomes (8):
  igb: Fix not adding filter elements to the list
  igb: Fix queue selection on MAC filters on i210 and i211
  igb: Enable the hardware traffic class feature bit for igb models
  igb: Add support for MAC address filters specifying source addresses
  igb: Enable nfc filters to specify MAC addresses
  igb: Add MAC address support for ethtool nftuple filters
  igb: Add the skeletons for tc-flower offloading
  igb: Add support for adding offloaded clsflower filters

 drivers/net/ethernet/intel/igb/e1000_defines.h |   2 +
 drivers/net/ethernet/intel/igb/igb.h           |  12 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c   |  65 +++++-
 drivers/net/ethernet/intel/igb/igb_main.c      | 306 ++++++++++++++++++++++++-
 4 files changed, 371 insertions(+), 14 deletions(-)

--
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v3 0/8] igb: offloading of receive filters
@ 2018-03-08  0:37 ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

Hi,

Changes from v3:
 - Addressed review comments from Aaron F. Brown and
   Jakub Kicinski;

Changes from v2:
 - Addressed review comments from Jakub Kicinski, mostly about coding
   style adjustments and more consistent error reporting;

Changes from v1:
 - Addressed review comments from Alexander Duyck and Florian
   Fainelli;
 - Adding and removing cls_flower filters are now proposed in the same
   patch;
 - cls_flower filters are kept in a separated list from "ethtool"
   filters (so that section of the original cover letter is no longer
   valid);
 - The patch adding support for ethtool filters is now independent from
   the rest of the series;

Original cover letter:

This series enables some ethtool and tc-flower filters to be offloaded
to igb-based network controllers. This is useful when the system
configurator want to steer kinds of traffic to a specific hardware
queue.

The first two commits are bug fixes.

The basis of this series is to export the internal API used to
configure address filters, so they can be used by ethtool, and
extending the functionality so an source address can be handled.

Then, we enable the tc-flower offloading implementation to re-use the
same infrastructure as ethtool, and storing them in the per-adapter
"nfc" (Network Filter Config?) list. But for consistency, for
destructive access they are separated, i.e. an filter added by
tc-flower can only be removed by tc-flower, but ethtool can read them
all.

Only support for VLAN Prio, Source and Destination MAC Address, and
Ethertype is enabled for now.

Open question:
  - igb is initialized with the number of traffic classes as 1, if we
  want to use multiple traffic classes we need to increase this value,
  the only way I could find is to use mqprio (for example). Should igb
  be initialized with, say, the number of queues as its "num_tc"?

Vinicius Costa Gomes (8):
  igb: Fix not adding filter elements to the list
  igb: Fix queue selection on MAC filters on i210 and i211
  igb: Enable the hardware traffic class feature bit for igb models
  igb: Add support for MAC address filters specifying source addresses
  igb: Enable nfc filters to specify MAC addresses
  igb: Add MAC address support for ethtool nftuple filters
  igb: Add the skeletons for tc-flower offloading
  igb: Add support for adding offloaded clsflower filters

 drivers/net/ethernet/intel/igb/e1000_defines.h |   2 +
 drivers/net/ethernet/intel/igb/igb.h           |  12 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c   |  65 +++++-
 drivers/net/ethernet/intel/igb/igb_main.c      | 306 ++++++++++++++++++++++++-
 4 files changed, 371 insertions(+), 14 deletions(-)

--
2.16.2

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

* [next-queue PATCH v4 1/8] igb: Fix not adding filter elements to the list
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

Because the order of the parameters passes to 'hlist_add_behind()' was
inverted, the 'parent' node was added "behind" the 'input', as input
is not in the list, this causes the 'input' node to be lost.

Fixes: 0e71def25281 ("igb: add support of RX network flow classification")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 606e6761758f..143f0bb34e4d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2864,7 +2864,7 @@ static int igb_update_ethtool_nfc_entry(struct igb_adapter *adapter,
 
 	/* add filter to the list */
 	if (parent)
-		hlist_add_behind(&parent->nfc_node, &input->nfc_node);
+		hlist_add_behind(&input->nfc_node, &parent->nfc_node);
 	else
 		hlist_add_head(&input->nfc_node, &adapter->nfc_filter_list);
 
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 1/8] igb: Fix not adding filter elements to the list
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

Because the order of the parameters passes to 'hlist_add_behind()' was
inverted, the 'parent' node was added "behind" the 'input', as input
is not in the list, this causes the 'input' node to be lost.

Fixes: 0e71def25281 ("igb: add support of RX network flow classification")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 606e6761758f..143f0bb34e4d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2864,7 +2864,7 @@ static int igb_update_ethtool_nfc_entry(struct igb_adapter *adapter,
 
 	/* add filter to the list */
 	if (parent)
-		hlist_add_behind(&parent->nfc_node, &input->nfc_node);
+		hlist_add_behind(&input->nfc_node, &parent->nfc_node);
 	else
 		hlist_add_head(&input->nfc_node, &adapter->nfc_filter_list);
 
-- 
2.16.2


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

* [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

On the RAH registers there are semantic differences on the meaning of
the "queue" parameter for traffic steering depending on the controller
model: there is the 82575 meaning, which "queue" means a RX Hardware
Queue, and the i350 meaning, where it is a reception pool.

The previous behaviour was having no effect for i210 and i211 based
controllers because the QSEL bit of the RAH register wasn't being set.

This patch separates the condition in discrete cases, so the different
handling is clearer.

Fixes: 83c21335c876 ("igb: improve MAC filter handling")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
 drivers/net/ethernet/intel/igb/igb_main.c      | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 83cabff1e0ab..573bf177fd08 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -490,6 +490,7 @@
  * manageability enabled, allowing us room for 15 multicast addresses.
  */
 #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
+#define E1000_RAH_QSEL_ENABLE 0x10000000
 #define E1000_RAL_MAC_ADDR_LEN 4
 #define E1000_RAH_MAC_ADDR_LEN 2
 #define E1000_RAH_POOL_MASK 0x03FC0000
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 715bb32e6901..eabedc6b6518 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter *adapter, u32 index)
 		if (is_valid_ether_addr(addr))
 			rar_high |= E1000_RAH_AV;
 
-		if (hw->mac.type == e1000_82575)
+		switch (hw->mac.type) {
+		case e1000_82575:
+		case e1000_i210:
+		case e1000_i211:
+			rar_high |= E1000_RAH_QSEL_ENABLE;
 			rar_high |= E1000_RAH_POOL_1 *
-				    adapter->mac_table[index].queue;
-		else
+				      adapter->mac_table[index].queue;
+			break;
+		default:
 			rar_high |= E1000_RAH_POOL_1 <<
-				    adapter->mac_table[index].queue;
+				adapter->mac_table[index].queue;
+			break;
+		}
 	}
 
 	wr32(E1000_RAL(index), rar_low);
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

On the RAH registers there are semantic differences on the meaning of
the "queue" parameter for traffic steering depending on the controller
model: there is the 82575 meaning, which "queue" means a RX Hardware
Queue, and the i350 meaning, where it is a reception pool.

The previous behaviour was having no effect for i210 and i211 based
controllers because the QSEL bit of the RAH register wasn't being set.

This patch separates the condition in discrete cases, so the different
handling is clearer.

Fixes: 83c21335c876 ("igb: improve MAC filter handling")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
 drivers/net/ethernet/intel/igb/igb_main.c      | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 83cabff1e0ab..573bf177fd08 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -490,6 +490,7 @@
  * manageability enabled, allowing us room for 15 multicast addresses.
  */
 #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
+#define E1000_RAH_QSEL_ENABLE 0x10000000
 #define E1000_RAL_MAC_ADDR_LEN 4
 #define E1000_RAH_MAC_ADDR_LEN 2
 #define E1000_RAH_POOL_MASK 0x03FC0000
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 715bb32e6901..eabedc6b6518 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter *adapter, u32 index)
 		if (is_valid_ether_addr(addr))
 			rar_high |= E1000_RAH_AV;
 
-		if (hw->mac.type == e1000_82575)
+		switch (hw->mac.type) {
+		case e1000_82575:
+		case e1000_i210:
+		case e1000_i211:
+			rar_high |= E1000_RAH_QSEL_ENABLE;
 			rar_high |= E1000_RAH_POOL_1 *
-				    adapter->mac_table[index].queue;
-		else
+				      adapter->mac_table[index].queue;
+			break;
+		default:
 			rar_high |= E1000_RAH_POOL_1 <<
-				    adapter->mac_table[index].queue;
+				adapter->mac_table[index].queue;
+			break;
+		}
 	}
 
 	wr32(E1000_RAL(index), rar_low);
-- 
2.16.2


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

* [next-queue PATCH v4 3/8] igb: Enable the hardware traffic class feature bit for igb models
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

This will allow functionality depending on the hardware being traffic
class aware to work. In particular the tc-flower offloading checks
verifies that this bit is set.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index eabedc6b6518..8684d5ed56e1 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2806,6 +2806,9 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (hw->mac.type >= e1000_82576)
 		netdev->features |= NETIF_F_SCTP_CRC;
 
+	if (hw->mac.type >= e1000_i350)
+		netdev->features |= NETIF_F_HW_TC;
+
 #define IGB_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
 				  NETIF_F_GSO_GRE_CSUM | \
 				  NETIF_F_GSO_IPXIP4 | \
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 3/8] igb: Enable the hardware traffic class feature bit for igb models
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

This will allow functionality depending on the hardware being traffic
class aware to work. In particular the tc-flower offloading checks
verifies that this bit is set.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index eabedc6b6518..8684d5ed56e1 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2806,6 +2806,9 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (hw->mac.type >= e1000_82576)
 		netdev->features |= NETIF_F_SCTP_CRC;
 
+	if (hw->mac.type >= e1000_i350)
+		netdev->features |= NETIF_F_HW_TC;
+
 #define IGB_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
 				  NETIF_F_GSO_GRE_CSUM | \
 				  NETIF_F_GSO_IPXIP4 | \
-- 
2.16.2


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

* [next-queue PATCH v4 4/8] igb: Add support for MAC address filters specifying source addresses
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

Makes it possible to direct packets to queues based on their source
address. Documents the expected usage of the 'flags' parameter.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
 drivers/net/ethernet/intel/igb/igb.h           |  1 +
 drivers/net/ethernet/intel/igb/igb_main.c      | 38 ++++++++++++++++++++++----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 573bf177fd08..c6f552de30dd 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -490,6 +490,7 @@
  * manageability enabled, allowing us room for 15 multicast addresses.
  */
 #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
+#define E1000_RAH_ASEL_SRC_ADDR 0x00010000
 #define E1000_RAH_QSEL_ENABLE 0x10000000
 #define E1000_RAL_MAC_ADDR_LEN 4
 #define E1000_RAH_MAC_ADDR_LEN 2
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 55d6f17d5799..4501b28ff7c5 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -473,6 +473,7 @@ struct igb_mac_addr {
 
 #define IGB_MAC_STATE_DEFAULT	0x1
 #define IGB_MAC_STATE_IN_USE	0x2
+#define IGB_MAC_STATE_SRC_ADDR  0x4
 
 /* board specific private data structure */
 struct igb_adapter {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8684d5ed56e1..76969467de31 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6843,8 +6843,13 @@ static void igb_set_default_mac_filter(struct igb_adapter *adapter)
 	igb_rar_set_index(adapter, 0);
 }
 
-static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
-			      const u8 queue)
+/* Add a MAC filter for 'addr' directing matching traffic to 'queue',
+ * 'flags' is used to indicate what kind of match is made, match is by
+ * default for the destination address, if matching by source address
+ * is desired the flag IGB_MAC_STATE_SRC_ADDR can be used.
+ */
+static int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+				    const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6864,7 +6869,7 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 
 		ether_addr_copy(adapter->mac_table[i].addr, addr);
 		adapter->mac_table[i].queue = queue;
-		adapter->mac_table[i].state |= IGB_MAC_STATE_IN_USE;
+		adapter->mac_table[i].state |= IGB_MAC_STATE_IN_USE | flags;
 
 		igb_rar_set_index(adapter, i);
 		return i;
@@ -6873,8 +6878,20 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	return -ENOSPC;
 }
 
-static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 			      const u8 queue)
+{
+	return igb_add_mac_filter_flags(adapter, addr, queue, 0);
+}
+
+/* Remove a MAC filter for 'addr' directing matching traffic to
+ * 'queue', 'flags' is used to indicate what kind of match need to be
+ * removed, match is by default for the destination address, if
+ * matching by source address is to be removed the flag
+ * IGB_MAC_STATE_SRC_ADDR can be used.
+ */
+static int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+				    const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6891,12 +6908,14 @@ static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	for (i = 0; i < rar_entries; i++) {
 		if (!(adapter->mac_table[i].state & IGB_MAC_STATE_IN_USE))
 			continue;
+		if ((adapter->mac_table[i].state & flags) != flags)
+			continue;
 		if (adapter->mac_table[i].queue != queue)
 			continue;
 		if (!ether_addr_equal(adapter->mac_table[i].addr, addr))
 			continue;
 
-		adapter->mac_table[i].state &= ~IGB_MAC_STATE_IN_USE;
+		adapter->mac_table[i].state = 0;
 		memset(adapter->mac_table[i].addr, 0, ETH_ALEN);
 		adapter->mac_table[i].queue = 0;
 
@@ -6907,6 +6926,12 @@ static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	return -ENOENT;
 }
 
+static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+			      const u8 queue)
+{
+	return igb_del_mac_filter_flags(adapter, addr, queue, 0);
+}
+
 static int igb_uc_sync(struct net_device *netdev, const unsigned char *addr)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
@@ -8750,6 +8775,9 @@ static void igb_rar_set_index(struct igb_adapter *adapter, u32 index)
 		if (is_valid_ether_addr(addr))
 			rar_high |= E1000_RAH_AV;
 
+		if (adapter->mac_table[index].state & IGB_MAC_STATE_SRC_ADDR)
+			rar_high |= E1000_RAH_ASEL_SRC_ADDR;
+
 		switch (hw->mac.type) {
 		case e1000_82575:
 		case e1000_i210:
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 4/8] igb: Add support for MAC address filters specifying source addresses
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

Makes it possible to direct packets to queues based on their source
address. Documents the expected usage of the 'flags' parameter.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
 drivers/net/ethernet/intel/igb/igb.h           |  1 +
 drivers/net/ethernet/intel/igb/igb_main.c      | 38 ++++++++++++++++++++++----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 573bf177fd08..c6f552de30dd 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -490,6 +490,7 @@
  * manageability enabled, allowing us room for 15 multicast addresses.
  */
 #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
+#define E1000_RAH_ASEL_SRC_ADDR 0x00010000
 #define E1000_RAH_QSEL_ENABLE 0x10000000
 #define E1000_RAL_MAC_ADDR_LEN 4
 #define E1000_RAH_MAC_ADDR_LEN 2
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 55d6f17d5799..4501b28ff7c5 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -473,6 +473,7 @@ struct igb_mac_addr {
 
 #define IGB_MAC_STATE_DEFAULT	0x1
 #define IGB_MAC_STATE_IN_USE	0x2
+#define IGB_MAC_STATE_SRC_ADDR  0x4
 
 /* board specific private data structure */
 struct igb_adapter {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8684d5ed56e1..76969467de31 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6843,8 +6843,13 @@ static void igb_set_default_mac_filter(struct igb_adapter *adapter)
 	igb_rar_set_index(adapter, 0);
 }
 
-static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
-			      const u8 queue)
+/* Add a MAC filter for 'addr' directing matching traffic to 'queue',
+ * 'flags' is used to indicate what kind of match is made, match is by
+ * default for the destination address, if matching by source address
+ * is desired the flag IGB_MAC_STATE_SRC_ADDR can be used.
+ */
+static int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+				    const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6864,7 +6869,7 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 
 		ether_addr_copy(adapter->mac_table[i].addr, addr);
 		adapter->mac_table[i].queue = queue;
-		adapter->mac_table[i].state |= IGB_MAC_STATE_IN_USE;
+		adapter->mac_table[i].state |= IGB_MAC_STATE_IN_USE | flags;
 
 		igb_rar_set_index(adapter, i);
 		return i;
@@ -6873,8 +6878,20 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	return -ENOSPC;
 }
 
-static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 			      const u8 queue)
+{
+	return igb_add_mac_filter_flags(adapter, addr, queue, 0);
+}
+
+/* Remove a MAC filter for 'addr' directing matching traffic to
+ * 'queue', 'flags' is used to indicate what kind of match need to be
+ * removed, match is by default for the destination address, if
+ * matching by source address is to be removed the flag
+ * IGB_MAC_STATE_SRC_ADDR can be used.
+ */
+static int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+				    const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6891,12 +6908,14 @@ static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	for (i = 0; i < rar_entries; i++) {
 		if (!(adapter->mac_table[i].state & IGB_MAC_STATE_IN_USE))
 			continue;
+		if ((adapter->mac_table[i].state & flags) != flags)
+			continue;
 		if (adapter->mac_table[i].queue != queue)
 			continue;
 		if (!ether_addr_equal(adapter->mac_table[i].addr, addr))
 			continue;
 
-		adapter->mac_table[i].state &= ~IGB_MAC_STATE_IN_USE;
+		adapter->mac_table[i].state = 0;
 		memset(adapter->mac_table[i].addr, 0, ETH_ALEN);
 		adapter->mac_table[i].queue = 0;
 
@@ -6907,6 +6926,12 @@ static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
 	return -ENOENT;
 }
 
+static int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+			      const u8 queue)
+{
+	return igb_del_mac_filter_flags(adapter, addr, queue, 0);
+}
+
 static int igb_uc_sync(struct net_device *netdev, const unsigned char *addr)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
@@ -8750,6 +8775,9 @@ static void igb_rar_set_index(struct igb_adapter *adapter, u32 index)
 		if (is_valid_ether_addr(addr))
 			rar_high |= E1000_RAH_AV;
 
+		if (adapter->mac_table[index].state & IGB_MAC_STATE_SRC_ADDR)
+			rar_high |= E1000_RAH_ASEL_SRC_ADDR;
+
 		switch (hw->mac.type) {
 		case e1000_82575:
 		case e1000_i210:
-- 
2.16.2


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

* [next-queue PATCH v4 5/8] igb: Enable nfc filters to specify MAC addresses
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

This allows igb_add_filter()/igb_erase_filter() to work on filters
that include MAC addresses (both source and destination).

For now, this only exposes the functionality, the next commit glues
ethtool into this. Later in this series, these APIs are used to allow
offloading of cls_flower filters.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  9 +++++++++
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 28 ++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c    |  8 ++++----
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 4501b28ff7c5..3b310b16e1d1 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -441,6 +441,8 @@ struct hwmon_buff {
 enum igb_filter_match_flags {
 	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
 	IGB_FILTER_FLAG_VLAN_TCI   = 0x2,
+	IGB_FILTER_FLAG_SRC_MAC_ADDR   = 0x4,
+	IGB_FILTER_FLAG_DST_MAC_ADDR   = 0x8,
 };
 
 #define IGB_MAX_RXNFC_FILTERS 16
@@ -455,6 +457,8 @@ struct igb_nfc_input {
 	u8 match_flags;
 	__be16 etype;
 	__be16 vlan_tci;
+	u8 src_addr[ETH_ALEN];
+	u8 dst_addr[ETH_ALEN];
 };
 
 struct igb_nfc_filter {
@@ -739,4 +743,9 @@ int igb_add_filter(struct igb_adapter *adapter,
 int igb_erase_filter(struct igb_adapter *adapter,
 		     struct igb_nfc_filter *input);
 
+int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags);
+int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags);
+
 #endif /* _IGB_H_ */
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 143f0bb34e4d..94fc9a4bed8b 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2775,6 +2775,25 @@ int igb_add_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input)
 			return err;
 	}
 
+	if (input->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR) {
+		err = igb_add_mac_filter_flags(adapter,
+					       input->filter.dst_addr,
+					       input->action, 0);
+		err = min_t(int, err, 0);
+		if (err)
+			return err;
+	}
+
+	if (input->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR) {
+		err = igb_add_mac_filter_flags(adapter,
+					       input->filter.src_addr,
+					       input->action,
+					       IGB_MAC_STATE_SRC_ADDR);
+		err = min_t(int, err, 0);
+		if (err)
+			return err;
+	}
+
 	if (input->filter.match_flags & IGB_FILTER_FLAG_VLAN_TCI)
 		err = igb_rxnfc_write_vlan_prio_filter(adapter, input);
 
@@ -2823,6 +2842,15 @@ int igb_erase_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input)
 		igb_clear_vlan_prio_filter(adapter,
 					   ntohs(input->filter.vlan_tci));
 
+	if (input->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR)
+		igb_del_mac_filter_flags(adapter, input->filter.src_addr,
+					 input->action,
+					 IGB_MAC_STATE_SRC_ADDR);
+
+	if (input->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR)
+		igb_del_mac_filter_flags(adapter, input->filter.dst_addr,
+					 input->action, 0);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 76969467de31..04307ef07e5a 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6848,8 +6848,8 @@ static void igb_set_default_mac_filter(struct igb_adapter *adapter)
  * default for the destination address, if matching by source address
  * is desired the flag IGB_MAC_STATE_SRC_ADDR can be used.
  */
-static int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
-				    const u8 queue, const u8 flags)
+int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6890,8 +6890,8 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
  * matching by source address is to be removed the flag
  * IGB_MAC_STATE_SRC_ADDR can be used.
  */
-static int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
-				    const u8 queue, const u8 flags)
+int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 5/8] igb: Enable nfc filters to specify MAC addresses
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

This allows igb_add_filter()/igb_erase_filter() to work on filters
that include MAC addresses (both source and destination).

For now, this only exposes the functionality, the next commit glues
ethtool into this. Later in this series, these APIs are used to allow
offloading of cls_flower filters.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  9 +++++++++
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 28 ++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c    |  8 ++++----
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 4501b28ff7c5..3b310b16e1d1 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -441,6 +441,8 @@ struct hwmon_buff {
 enum igb_filter_match_flags {
 	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
 	IGB_FILTER_FLAG_VLAN_TCI   = 0x2,
+	IGB_FILTER_FLAG_SRC_MAC_ADDR   = 0x4,
+	IGB_FILTER_FLAG_DST_MAC_ADDR   = 0x8,
 };
 
 #define IGB_MAX_RXNFC_FILTERS 16
@@ -455,6 +457,8 @@ struct igb_nfc_input {
 	u8 match_flags;
 	__be16 etype;
 	__be16 vlan_tci;
+	u8 src_addr[ETH_ALEN];
+	u8 dst_addr[ETH_ALEN];
 };
 
 struct igb_nfc_filter {
@@ -739,4 +743,9 @@ int igb_add_filter(struct igb_adapter *adapter,
 int igb_erase_filter(struct igb_adapter *adapter,
 		     struct igb_nfc_filter *input);
 
+int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags);
+int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags);
+
 #endif /* _IGB_H_ */
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 143f0bb34e4d..94fc9a4bed8b 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2775,6 +2775,25 @@ int igb_add_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input)
 			return err;
 	}
 
+	if (input->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR) {
+		err = igb_add_mac_filter_flags(adapter,
+					       input->filter.dst_addr,
+					       input->action, 0);
+		err = min_t(int, err, 0);
+		if (err)
+			return err;
+	}
+
+	if (input->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR) {
+		err = igb_add_mac_filter_flags(adapter,
+					       input->filter.src_addr,
+					       input->action,
+					       IGB_MAC_STATE_SRC_ADDR);
+		err = min_t(int, err, 0);
+		if (err)
+			return err;
+	}
+
 	if (input->filter.match_flags & IGB_FILTER_FLAG_VLAN_TCI)
 		err = igb_rxnfc_write_vlan_prio_filter(adapter, input);
 
@@ -2823,6 +2842,15 @@ int igb_erase_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input)
 		igb_clear_vlan_prio_filter(adapter,
 					   ntohs(input->filter.vlan_tci));
 
+	if (input->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR)
+		igb_del_mac_filter_flags(adapter, input->filter.src_addr,
+					 input->action,
+					 IGB_MAC_STATE_SRC_ADDR);
+
+	if (input->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR)
+		igb_del_mac_filter_flags(adapter, input->filter.dst_addr,
+					 input->action, 0);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 76969467de31..04307ef07e5a 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6848,8 +6848,8 @@ static void igb_set_default_mac_filter(struct igb_adapter *adapter)
  * default for the destination address, if matching by source address
  * is desired the flag IGB_MAC_STATE_SRC_ADDR can be used.
  */
-static int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
-				    const u8 queue, const u8 flags)
+int igb_add_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
@@ -6890,8 +6890,8 @@ static int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
  * matching by source address is to be removed the flag
  * IGB_MAC_STATE_SRC_ADDR can be used.
  */
-static int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
-				    const u8 queue, const u8 flags)
+int igb_del_mac_filter_flags(struct igb_adapter *adapter, const u8 *addr,
+			     const u8 queue, const u8 flags)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count -
-- 
2.16.2


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

* [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

This adds the capability of configuring the queue steering of arriving
packets based on their source and destination MAC addresses.

In practical terms this adds support for the following use cases,
characterized by these examples:

$ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
(this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
to the RX queue 0)

$ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
(this will direct packets with source address "44:44:44:44:44:44" to
the RX queue 3)

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 35 ++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 94fc9a4bed8b..3f98299d4cd0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2494,6 +2494,23 @@ static int igb_get_ethtool_nfc_entry(struct igb_adapter *adapter,
 			fsp->h_ext.vlan_tci = rule->filter.vlan_tci;
 			fsp->m_ext.vlan_tci = htons(VLAN_PRIO_MASK);
 		}
+		if (rule->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR) {
+			ether_addr_copy(fsp->h_u.ether_spec.h_dest,
+					rule->filter.dst_addr);
+			/* As we only support matching by the full
+			 * mask, return the mask to userspace
+			 */
+			eth_broadcast_addr(fsp->m_u.ether_spec.h_dest);
+		}
+		if (rule->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR) {
+			ether_addr_copy(fsp->h_u.ether_spec.h_source,
+					rule->filter.src_addr);
+			/* As we only support matching by the full
+			 * mask, return the mask to userspace
+			 */
+			eth_broadcast_addr(fsp->m_u.ether_spec.h_source);
+		}
+
 		return 0;
 	}
 	return -EINVAL;
@@ -2932,10 +2949,6 @@ static int igb_add_ethtool_nfc_entry(struct igb_adapter *adapter,
 	if ((fsp->flow_type & ~FLOW_EXT) != ETHER_FLOW)
 		return -EINVAL;
 
-	if (fsp->m_u.ether_spec.h_proto != ETHER_TYPE_FULL_MASK &&
-	    fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK))
-		return -EINVAL;
-
 	input = kzalloc(sizeof(*input), GFP_KERNEL);
 	if (!input)
 		return -ENOMEM;
@@ -2945,6 +2958,20 @@ static int igb_add_ethtool_nfc_entry(struct igb_adapter *adapter,
 		input->filter.match_flags = IGB_FILTER_FLAG_ETHER_TYPE;
 	}
 
+	/* Only support matching addresses by the full mask */
+	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_source)) {
+		input->filter.match_flags |= IGB_FILTER_FLAG_SRC_MAC_ADDR;
+		ether_addr_copy(input->filter.src_addr,
+				fsp->h_u.ether_spec.h_source);
+	}
+
+	/* Only support matching addresses by the full mask */
+	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_dest)) {
+		input->filter.match_flags |= IGB_FILTER_FLAG_DST_MAC_ADDR;
+		ether_addr_copy(input->filter.dst_addr,
+				fsp->h_u.ether_spec.h_dest);
+	}
+
 	if ((fsp->flow_type & FLOW_EXT) && fsp->m_ext.vlan_tci) {
 		if (fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK)) {
 			err = -EINVAL;
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

This adds the capability of configuring the queue steering of arriving
packets based on their source and destination MAC addresses.

In practical terms this adds support for the following use cases,
characterized by these examples:

$ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
(this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
to the RX queue 0)

$ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
(this will direct packets with source address "44:44:44:44:44:44" to
the RX queue 3)

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 35 ++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 94fc9a4bed8b..3f98299d4cd0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2494,6 +2494,23 @@ static int igb_get_ethtool_nfc_entry(struct igb_adapter *adapter,
 			fsp->h_ext.vlan_tci = rule->filter.vlan_tci;
 			fsp->m_ext.vlan_tci = htons(VLAN_PRIO_MASK);
 		}
+		if (rule->filter.match_flags & IGB_FILTER_FLAG_DST_MAC_ADDR) {
+			ether_addr_copy(fsp->h_u.ether_spec.h_dest,
+					rule->filter.dst_addr);
+			/* As we only support matching by the full
+			 * mask, return the mask to userspace
+			 */
+			eth_broadcast_addr(fsp->m_u.ether_spec.h_dest);
+		}
+		if (rule->filter.match_flags & IGB_FILTER_FLAG_SRC_MAC_ADDR) {
+			ether_addr_copy(fsp->h_u.ether_spec.h_source,
+					rule->filter.src_addr);
+			/* As we only support matching by the full
+			 * mask, return the mask to userspace
+			 */
+			eth_broadcast_addr(fsp->m_u.ether_spec.h_source);
+		}
+
 		return 0;
 	}
 	return -EINVAL;
@@ -2932,10 +2949,6 @@ static int igb_add_ethtool_nfc_entry(struct igb_adapter *adapter,
 	if ((fsp->flow_type & ~FLOW_EXT) != ETHER_FLOW)
 		return -EINVAL;
 
-	if (fsp->m_u.ether_spec.h_proto != ETHER_TYPE_FULL_MASK &&
-	    fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK))
-		return -EINVAL;
-
 	input = kzalloc(sizeof(*input), GFP_KERNEL);
 	if (!input)
 		return -ENOMEM;
@@ -2945,6 +2958,20 @@ static int igb_add_ethtool_nfc_entry(struct igb_adapter *adapter,
 		input->filter.match_flags = IGB_FILTER_FLAG_ETHER_TYPE;
 	}
 
+	/* Only support matching addresses by the full mask */
+	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_source)) {
+		input->filter.match_flags |= IGB_FILTER_FLAG_SRC_MAC_ADDR;
+		ether_addr_copy(input->filter.src_addr,
+				fsp->h_u.ether_spec.h_source);
+	}
+
+	/* Only support matching addresses by the full mask */
+	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_dest)) {
+		input->filter.match_flags |= IGB_FILTER_FLAG_DST_MAC_ADDR;
+		ether_addr_copy(input->filter.dst_addr,
+				fsp->h_u.ether_spec.h_dest);
+	}
+
 	if ((fsp->flow_type & FLOW_EXT) && fsp->m_ext.vlan_tci) {
 		if (fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK)) {
 			err = -EINVAL;
-- 
2.16.2


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

* [next-queue PATCH v4 7/8] igb: Add the skeletons for tc-flower offloading
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

This adds basic functions needed to implement offloading for filters
created by tc-flower.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 66 +++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 04307ef07e5a..7762dd5f270d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -35,6 +35,7 @@
 #include <net/checksum.h>
 #include <net/ip6_checksum.h>
 #include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
 #include <linux/net_tstamp.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
@@ -2497,6 +2498,69 @@ static int igb_offload_cbs(struct igb_adapter *adapter,
 	return 0;
 }
 
+static int igb_configure_clsflower(struct igb_adapter *adapter,
+				   struct tc_cls_flower_offload *cls_flower)
+{
+	return -EOPNOTSUPP;
+}
+
+static int igb_delete_clsflower(struct igb_adapter *adapter,
+				struct tc_cls_flower_offload *cls_flower)
+{
+	return -EOPNOTSUPP;
+}
+
+static int igb_setup_tc_cls_flower(struct igb_adapter *adapter,
+				   struct tc_cls_flower_offload *cls_flower)
+{
+	switch (cls_flower->command) {
+	case TC_CLSFLOWER_REPLACE:
+		return igb_configure_clsflower(adapter, cls_flower);
+	case TC_CLSFLOWER_DESTROY:
+		return igb_delete_clsflower(adapter, cls_flower);
+	case TC_CLSFLOWER_STATS:
+		return -EOPNOTSUPP;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int igb_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
+				 void *cb_priv)
+{
+	struct igb_adapter *adapter = cb_priv;
+
+	if (!tc_cls_can_offload_and_chain0(adapter->netdev, type_data))
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case TC_SETUP_CLSFLOWER:
+		return igb_setup_tc_cls_flower(adapter, type_data);
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int igb_setup_tc_block(struct igb_adapter *adapter,
+			      struct tc_block_offload *f)
+{
+	if (f->binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+		return -EOPNOTSUPP;
+
+	switch (f->command) {
+	case TC_BLOCK_BIND:
+		return tcf_block_cb_register(f->block, igb_setup_tc_block_cb,
+					     adapter, adapter);
+	case TC_BLOCK_UNBIND:
+		tcf_block_cb_unregister(f->block, igb_setup_tc_block_cb,
+					adapter);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
 			void *type_data)
 {
@@ -2505,6 +2569,8 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	switch (type) {
 	case TC_SETUP_QDISC_CBS:
 		return igb_offload_cbs(adapter, type_data);
+	case TC_SETUP_BLOCK:
+		return igb_setup_tc_block(adapter, type_data);
 
 	default:
 		return -EOPNOTSUPP;
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 7/8] igb: Add the skeletons for tc-flower offloading
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

This adds basic functions needed to implement offloading for filters
created by tc-flower.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 66 +++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 04307ef07e5a..7762dd5f270d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -35,6 +35,7 @@
 #include <net/checksum.h>
 #include <net/ip6_checksum.h>
 #include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
 #include <linux/net_tstamp.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
@@ -2497,6 +2498,69 @@ static int igb_offload_cbs(struct igb_adapter *adapter,
 	return 0;
 }
 
+static int igb_configure_clsflower(struct igb_adapter *adapter,
+				   struct tc_cls_flower_offload *cls_flower)
+{
+	return -EOPNOTSUPP;
+}
+
+static int igb_delete_clsflower(struct igb_adapter *adapter,
+				struct tc_cls_flower_offload *cls_flower)
+{
+	return -EOPNOTSUPP;
+}
+
+static int igb_setup_tc_cls_flower(struct igb_adapter *adapter,
+				   struct tc_cls_flower_offload *cls_flower)
+{
+	switch (cls_flower->command) {
+	case TC_CLSFLOWER_REPLACE:
+		return igb_configure_clsflower(adapter, cls_flower);
+	case TC_CLSFLOWER_DESTROY:
+		return igb_delete_clsflower(adapter, cls_flower);
+	case TC_CLSFLOWER_STATS:
+		return -EOPNOTSUPP;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int igb_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
+				 void *cb_priv)
+{
+	struct igb_adapter *adapter = cb_priv;
+
+	if (!tc_cls_can_offload_and_chain0(adapter->netdev, type_data))
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case TC_SETUP_CLSFLOWER:
+		return igb_setup_tc_cls_flower(adapter, type_data);
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int igb_setup_tc_block(struct igb_adapter *adapter,
+			      struct tc_block_offload *f)
+{
+	if (f->binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+		return -EOPNOTSUPP;
+
+	switch (f->command) {
+	case TC_BLOCK_BIND:
+		return tcf_block_cb_register(f->block, igb_setup_tc_block_cb,
+					     adapter, adapter);
+	case TC_BLOCK_UNBIND:
+		tcf_block_cb_unregister(f->block, igb_setup_tc_block_cb,
+					adapter);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
 			void *type_data)
 {
@@ -2505,6 +2569,8 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	switch (type) {
 	case TC_SETUP_QDISC_CBS:
 		return igb_offload_cbs(adapter, type_data);
+	case TC_SETUP_BLOCK:
+		return igb_setup_tc_block(adapter, type_data);
 
 	default:
 		return -EOPNOTSUPP;
-- 
2.16.2


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

* [next-queue PATCH v4 8/8] igb: Add support for adding offloaded clsflower filters
  2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Vinicius Costa Gomes, jeffrey.t.kirsher, netdev, jesus.sanchez-palencia

This allows filters added by tc-flower and specifying MAC addresses,
Ethernet types, and the VLAN priority field, to be offloaded to the
controller.

This reuses most of the infrastructure used by ethtool, but clsflower
filters are kept in a separated list, so they are invisible to
ethtool.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |   2 +
 drivers/net/ethernet/intel/igb/igb_main.c | 188 +++++++++++++++++++++++++++++-
 2 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 3b310b16e1d1..1874c4635d54 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -464,6 +464,7 @@ struct igb_nfc_input {
 struct igb_nfc_filter {
 	struct hlist_node nfc_node;
 	struct igb_nfc_input filter;
+	unsigned long cookie;
 	u16 etype_reg_index;
 	u16 sw_idx;
 	u16 action;
@@ -602,6 +603,7 @@ struct igb_adapter {
 
 	/* RX network flow classification support */
 	struct hlist_head nfc_filter_list;
+	struct hlist_head cls_flower_list;
 	unsigned int nfc_filter_count;
 	/* lock for RX network flow classification filter */
 	spinlock_t nfc_lock;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 7762dd5f270d..d7d86a7955bd 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2498,16 +2498,197 @@ static int igb_offload_cbs(struct igb_adapter *adapter,
 	return 0;
 }
 
+#define ETHER_TYPE_FULL_MASK ((__force __be16)~0)
+#define VLAN_PRIO_FULL_MASK (0x07)
+
+static int igb_parse_cls_flower(struct igb_adapter *adapter,
+				struct tc_cls_flower_offload *f,
+				int traffic_class,
+				struct igb_nfc_filter *input)
+{
+	struct netlink_ext_ack *extack = f->common.extack;
+
+	if (f->dissector->used_keys &
+	    ~(BIT(FLOW_DISSECTOR_KEY_BASIC) |
+	      BIT(FLOW_DISSECTOR_KEY_CONTROL) |
+	      BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) |
+	      BIT(FLOW_DISSECTOR_KEY_VLAN))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Unsupported key used, only BASIC, CONTROL, ETH_ADDRS and VLAN are supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
+		struct flow_dissector_key_eth_addrs *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_ETH_ADDRS,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_ETH_ADDRS,
+						 f->mask);
+
+		if (!is_zero_ether_addr(mask->dst)) {
+			if (!is_broadcast_ether_addr(mask->dst)) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full masks are supported for destination MAC address");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |=
+				IGB_FILTER_FLAG_DST_MAC_ADDR;
+			ether_addr_copy(input->filter.dst_addr, key->dst);
+		}
+
+		if (!is_zero_ether_addr(mask->src)) {
+			if (!is_broadcast_ether_addr(mask->src)) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full masks are supported for source MAC address");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |=
+				IGB_FILTER_FLAG_SRC_MAC_ADDR;
+			ether_addr_copy(input->filter.src_addr, key->src);
+		}
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_BASIC)) {
+		struct flow_dissector_key_basic *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_BASIC,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_BASIC,
+						 f->mask);
+
+		if (mask->n_proto) {
+			if (mask->n_proto != ETHER_TYPE_FULL_MASK) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full mask is supported for EtherType filter");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |= IGB_FILTER_FLAG_ETHER_TYPE;
+			input->filter.etype = key->n_proto;
+		}
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_VLAN)) {
+		struct flow_dissector_key_vlan *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_VLAN,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_VLAN,
+						 f->mask);
+
+		if (mask->vlan_priority) {
+			if (mask->vlan_priority != VLAN_PRIO_FULL_MASK) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full mask is supported for VLAN priority");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |= IGB_FILTER_FLAG_VLAN_TCI;
+			input->filter.vlan_tci = key->vlan_priority;
+		}
+	}
+
+	input->action = traffic_class;
+	input->cookie = f->cookie;
+
+	return 0;
+}
+
 static int igb_configure_clsflower(struct igb_adapter *adapter,
 				   struct tc_cls_flower_offload *cls_flower)
 {
-	return -EOPNOTSUPP;
+	struct netlink_ext_ack *extack = cls_flower->common.extack;
+	struct igb_nfc_filter *filter, *f;
+	int err, tc;
+
+	tc = tc_classid_to_hwtc(adapter->netdev, cls_flower->classid);
+	if (tc < 0) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid traffic class");
+		return -EINVAL;
+	}
+
+	filter = kzalloc(sizeof(*filter), GFP_KERNEL);
+	if (!filter)
+		return -ENOMEM;
+
+	err = igb_parse_cls_flower(adapter, cls_flower, tc, filter);
+	if (err < 0)
+		goto err_parse;
+
+	spin_lock(&adapter->nfc_lock);
+
+	hlist_for_each_entry(f, &adapter->nfc_filter_list, nfc_node) {
+		if (!memcmp(&f->filter, &filter->filter, sizeof(f->filter))) {
+			err = -EEXIST;
+			NL_SET_ERR_MSG_MOD(extack,
+					   "This filter is already set in ethtool");
+			goto err_locked;
+		}
+	}
+
+	hlist_for_each_entry(f, &adapter->cls_flower_list, nfc_node) {
+		if (!memcmp(&f->filter, &filter->filter, sizeof(f->filter))) {
+			err = -EEXIST;
+			NL_SET_ERR_MSG_MOD(extack,
+					   "This filter is already set in cls_flower");
+			goto err_locked;
+		}
+	}
+
+	err = igb_add_filter(adapter, filter);
+	if (err < 0) {
+		NL_SET_ERR_MSG_MOD(extack, "Could not add filter to the adapter");
+		goto err_locked;
+	}
+
+	hlist_add_head(&filter->nfc_node, &adapter->cls_flower_list);
+
+	spin_unlock(&adapter->nfc_lock);
+
+	return 0;
+
+err_locked:
+	spin_unlock(&adapter->nfc_lock);
+
+err_parse:
+	kfree(filter);
+
+	return err;
 }
 
 static int igb_delete_clsflower(struct igb_adapter *adapter,
 				struct tc_cls_flower_offload *cls_flower)
 {
-	return -EOPNOTSUPP;
+	struct igb_nfc_filter *filter;
+	int err;
+
+	spin_lock(&adapter->nfc_lock);
+
+	hlist_for_each_entry(filter, &adapter->cls_flower_list, nfc_node)
+		if (filter->cookie == cls_flower->cookie)
+			break;
+
+	if (!filter) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	err = igb_erase_filter(adapter, filter);
+	if (err < 0)
+		goto out;
+
+	hlist_del(&filter->nfc_node);
+	kfree(filter);
+
+out:
+	spin_unlock(&adapter->nfc_lock);
+
+	return err;
 }
 
 static int igb_setup_tc_cls_flower(struct igb_adapter *adapter,
@@ -9294,6 +9475,9 @@ static void igb_nfc_filter_exit(struct igb_adapter *adapter)
 	hlist_for_each_entry(rule, &adapter->nfc_filter_list, nfc_node)
 		igb_erase_filter(adapter, rule);
 
+	hlist_for_each_entry(rule, &adapter->cls_flower_list, nfc_node)
+		igb_erase_filter(adapter, rule);
+
 	spin_unlock(&adapter->nfc_lock);
 }
 
-- 
2.16.2

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

* [Intel-wired-lan] [next-queue PATCH v4 8/8] igb: Add support for adding offloaded clsflower filters
@ 2018-03-08  0:37   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-08  0:37 UTC (permalink / raw)
  To: intel-wired-lan

This allows filters added by tc-flower and specifying MAC addresses,
Ethernet types, and the VLAN priority field, to be offloaded to the
controller.

This reuses most of the infrastructure used by ethtool, but clsflower
filters are kept in a separated list, so they are invisible to
ethtool.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |   2 +
 drivers/net/ethernet/intel/igb/igb_main.c | 188 +++++++++++++++++++++++++++++-
 2 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 3b310b16e1d1..1874c4635d54 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -464,6 +464,7 @@ struct igb_nfc_input {
 struct igb_nfc_filter {
 	struct hlist_node nfc_node;
 	struct igb_nfc_input filter;
+	unsigned long cookie;
 	u16 etype_reg_index;
 	u16 sw_idx;
 	u16 action;
@@ -602,6 +603,7 @@ struct igb_adapter {
 
 	/* RX network flow classification support */
 	struct hlist_head nfc_filter_list;
+	struct hlist_head cls_flower_list;
 	unsigned int nfc_filter_count;
 	/* lock for RX network flow classification filter */
 	spinlock_t nfc_lock;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 7762dd5f270d..d7d86a7955bd 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2498,16 +2498,197 @@ static int igb_offload_cbs(struct igb_adapter *adapter,
 	return 0;
 }
 
+#define ETHER_TYPE_FULL_MASK ((__force __be16)~0)
+#define VLAN_PRIO_FULL_MASK (0x07)
+
+static int igb_parse_cls_flower(struct igb_adapter *adapter,
+				struct tc_cls_flower_offload *f,
+				int traffic_class,
+				struct igb_nfc_filter *input)
+{
+	struct netlink_ext_ack *extack = f->common.extack;
+
+	if (f->dissector->used_keys &
+	    ~(BIT(FLOW_DISSECTOR_KEY_BASIC) |
+	      BIT(FLOW_DISSECTOR_KEY_CONTROL) |
+	      BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) |
+	      BIT(FLOW_DISSECTOR_KEY_VLAN))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Unsupported key used, only BASIC, CONTROL, ETH_ADDRS and VLAN are supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
+		struct flow_dissector_key_eth_addrs *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_ETH_ADDRS,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_ETH_ADDRS,
+						 f->mask);
+
+		if (!is_zero_ether_addr(mask->dst)) {
+			if (!is_broadcast_ether_addr(mask->dst)) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full masks are supported for destination MAC address");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |=
+				IGB_FILTER_FLAG_DST_MAC_ADDR;
+			ether_addr_copy(input->filter.dst_addr, key->dst);
+		}
+
+		if (!is_zero_ether_addr(mask->src)) {
+			if (!is_broadcast_ether_addr(mask->src)) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full masks are supported for source MAC address");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |=
+				IGB_FILTER_FLAG_SRC_MAC_ADDR;
+			ether_addr_copy(input->filter.src_addr, key->src);
+		}
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_BASIC)) {
+		struct flow_dissector_key_basic *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_BASIC,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_BASIC,
+						 f->mask);
+
+		if (mask->n_proto) {
+			if (mask->n_proto != ETHER_TYPE_FULL_MASK) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full mask is supported for EtherType filter");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |= IGB_FILTER_FLAG_ETHER_TYPE;
+			input->filter.etype = key->n_proto;
+		}
+	}
+
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_VLAN)) {
+		struct flow_dissector_key_vlan *key, *mask;
+
+		key = skb_flow_dissector_target(f->dissector,
+						FLOW_DISSECTOR_KEY_VLAN,
+						f->key);
+		mask = skb_flow_dissector_target(f->dissector,
+						 FLOW_DISSECTOR_KEY_VLAN,
+						 f->mask);
+
+		if (mask->vlan_priority) {
+			if (mask->vlan_priority != VLAN_PRIO_FULL_MASK) {
+				NL_SET_ERR_MSG_MOD(extack, "Only full mask is supported for VLAN priority");
+				return -EINVAL;
+			}
+
+			input->filter.match_flags |= IGB_FILTER_FLAG_VLAN_TCI;
+			input->filter.vlan_tci = key->vlan_priority;
+		}
+	}
+
+	input->action = traffic_class;
+	input->cookie = f->cookie;
+
+	return 0;
+}
+
 static int igb_configure_clsflower(struct igb_adapter *adapter,
 				   struct tc_cls_flower_offload *cls_flower)
 {
-	return -EOPNOTSUPP;
+	struct netlink_ext_ack *extack = cls_flower->common.extack;
+	struct igb_nfc_filter *filter, *f;
+	int err, tc;
+
+	tc = tc_classid_to_hwtc(adapter->netdev, cls_flower->classid);
+	if (tc < 0) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid traffic class");
+		return -EINVAL;
+	}
+
+	filter = kzalloc(sizeof(*filter), GFP_KERNEL);
+	if (!filter)
+		return -ENOMEM;
+
+	err = igb_parse_cls_flower(adapter, cls_flower, tc, filter);
+	if (err < 0)
+		goto err_parse;
+
+	spin_lock(&adapter->nfc_lock);
+
+	hlist_for_each_entry(f, &adapter->nfc_filter_list, nfc_node) {
+		if (!memcmp(&f->filter, &filter->filter, sizeof(f->filter))) {
+			err = -EEXIST;
+			NL_SET_ERR_MSG_MOD(extack,
+					   "This filter is already set in ethtool");
+			goto err_locked;
+		}
+	}
+
+	hlist_for_each_entry(f, &adapter->cls_flower_list, nfc_node) {
+		if (!memcmp(&f->filter, &filter->filter, sizeof(f->filter))) {
+			err = -EEXIST;
+			NL_SET_ERR_MSG_MOD(extack,
+					   "This filter is already set in cls_flower");
+			goto err_locked;
+		}
+	}
+
+	err = igb_add_filter(adapter, filter);
+	if (err < 0) {
+		NL_SET_ERR_MSG_MOD(extack, "Could not add filter to the adapter");
+		goto err_locked;
+	}
+
+	hlist_add_head(&filter->nfc_node, &adapter->cls_flower_list);
+
+	spin_unlock(&adapter->nfc_lock);
+
+	return 0;
+
+err_locked:
+	spin_unlock(&adapter->nfc_lock);
+
+err_parse:
+	kfree(filter);
+
+	return err;
 }
 
 static int igb_delete_clsflower(struct igb_adapter *adapter,
 				struct tc_cls_flower_offload *cls_flower)
 {
-	return -EOPNOTSUPP;
+	struct igb_nfc_filter *filter;
+	int err;
+
+	spin_lock(&adapter->nfc_lock);
+
+	hlist_for_each_entry(filter, &adapter->cls_flower_list, nfc_node)
+		if (filter->cookie == cls_flower->cookie)
+			break;
+
+	if (!filter) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	err = igb_erase_filter(adapter, filter);
+	if (err < 0)
+		goto out;
+
+	hlist_del(&filter->nfc_node);
+	kfree(filter);
+
+out:
+	spin_unlock(&adapter->nfc_lock);
+
+	return err;
 }
 
 static int igb_setup_tc_cls_flower(struct igb_adapter *adapter,
@@ -9294,6 +9475,9 @@ static void igb_nfc_filter_exit(struct igb_adapter *adapter)
 	hlist_for_each_entry(rule, &adapter->nfc_filter_list, nfc_node)
 		igb_erase_filter(adapter, rule);
 
+	hlist_for_each_entry(rule, &adapter->cls_flower_list, nfc_node)
+		igb_erase_filter(adapter, rule);
+
 	spin_unlock(&adapter->nfc_lock);
 }
 
-- 
2.16.2


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

* RE: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-14  3:04     ` Brown, Aaron F
  -1 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-14  3:04 UTC (permalink / raw)
  To: Gomes, Vinicius, intel-wired-lan; +Cc: netdev, Sanchez-Palencia, Jesus

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Vinicius Costa Gomes
> Sent: Wednesday, March 7, 2018 4:37 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia@intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
> support for ethtool nftuple filters
> 
> This adds the capability of configuring the queue steering of arriving
> packets based on their source and destination MAC addresses.
> 
> In practical terms this adds support for the following use cases,
> characterized by these examples:
> 
> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
> to the RX queue 0)
> 
> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
> (this will direct packets with source address "44:44:44:44:44:44" to
> the RX queue 3)

This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.

With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.

> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 35
> ++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 94fc9a4bed8b..3f98299d4cd0 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -2494,6 +2494,23 @@ static int igb_get_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  			fsp->h_ext.vlan_tci = rule->filter.vlan_tci;
>  			fsp->m_ext.vlan_tci = htons(VLAN_PRIO_MASK);
>  		}
> +		if (rule->filter.match_flags &
> IGB_FILTER_FLAG_DST_MAC_ADDR) {
> +			ether_addr_copy(fsp->h_u.ether_spec.h_dest,
> +					rule->filter.dst_addr);
> +			/* As we only support matching by the full
> +			 * mask, return the mask to userspace
> +			 */
> +			eth_broadcast_addr(fsp->m_u.ether_spec.h_dest);
> +		}
> +		if (rule->filter.match_flags &
> IGB_FILTER_FLAG_SRC_MAC_ADDR) {
> +			ether_addr_copy(fsp->h_u.ether_spec.h_source,
> +					rule->filter.src_addr);
> +			/* As we only support matching by the full
> +			 * mask, return the mask to userspace
> +			 */
> +			eth_broadcast_addr(fsp-
> >m_u.ether_spec.h_source);
> +		}
> +
>  		return 0;
>  	}
>  	return -EINVAL;
> @@ -2932,10 +2949,6 @@ static int igb_add_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  	if ((fsp->flow_type & ~FLOW_EXT) != ETHER_FLOW)
>  		return -EINVAL;
> 
> -	if (fsp->m_u.ether_spec.h_proto != ETHER_TYPE_FULL_MASK &&
> -	    fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK))
> -		return -EINVAL;
> -
>  	input = kzalloc(sizeof(*input), GFP_KERNEL);
>  	if (!input)
>  		return -ENOMEM;
> @@ -2945,6 +2958,20 @@ static int igb_add_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  		input->filter.match_flags = IGB_FILTER_FLAG_ETHER_TYPE;
>  	}
> 
> +	/* Only support matching addresses by the full mask */
> +	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_source)) {
> +		input->filter.match_flags |=
> IGB_FILTER_FLAG_SRC_MAC_ADDR;
> +		ether_addr_copy(input->filter.src_addr,
> +				fsp->h_u.ether_spec.h_source);
> +	}
> +
> +	/* Only support matching addresses by the full mask */
> +	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_dest)) {
> +		input->filter.match_flags |=
> IGB_FILTER_FLAG_DST_MAC_ADDR;
> +		ether_addr_copy(input->filter.dst_addr,
> +				fsp->h_u.ether_spec.h_dest);
> +	}
> +
>  	if ((fsp->flow_type & FLOW_EXT) && fsp->m_ext.vlan_tci) {
>  		if (fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK)) {
>  			err = -EINVAL;
> --
> 2.16.2
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-14  3:04     ` Brown, Aaron F
  0 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-14  3:04 UTC (permalink / raw)
  To: intel-wired-lan

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Vinicius Costa Gomes
> Sent: Wednesday, March 7, 2018 4:37 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia at intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
> support for ethtool nftuple filters
> 
> This adds the capability of configuring the queue steering of arriving
> packets based on their source and destination MAC addresses.
> 
> In practical terms this adds support for the following use cases,
> characterized by these examples:
> 
> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
> to the RX queue 0)
> 
> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
> (this will direct packets with source address "44:44:44:44:44:44" to
> the RX queue 3)

This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.

With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.

> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 35
> ++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 94fc9a4bed8b..3f98299d4cd0 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -2494,6 +2494,23 @@ static int igb_get_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  			fsp->h_ext.vlan_tci = rule->filter.vlan_tci;
>  			fsp->m_ext.vlan_tci = htons(VLAN_PRIO_MASK);
>  		}
> +		if (rule->filter.match_flags &
> IGB_FILTER_FLAG_DST_MAC_ADDR) {
> +			ether_addr_copy(fsp->h_u.ether_spec.h_dest,
> +					rule->filter.dst_addr);
> +			/* As we only support matching by the full
> +			 * mask, return the mask to userspace
> +			 */
> +			eth_broadcast_addr(fsp->m_u.ether_spec.h_dest);
> +		}
> +		if (rule->filter.match_flags &
> IGB_FILTER_FLAG_SRC_MAC_ADDR) {
> +			ether_addr_copy(fsp->h_u.ether_spec.h_source,
> +					rule->filter.src_addr);
> +			/* As we only support matching by the full
> +			 * mask, return the mask to userspace
> +			 */
> +			eth_broadcast_addr(fsp-
> >m_u.ether_spec.h_source);
> +		}
> +
>  		return 0;
>  	}
>  	return -EINVAL;
> @@ -2932,10 +2949,6 @@ static int igb_add_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  	if ((fsp->flow_type & ~FLOW_EXT) != ETHER_FLOW)
>  		return -EINVAL;
> 
> -	if (fsp->m_u.ether_spec.h_proto != ETHER_TYPE_FULL_MASK &&
> -	    fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK))
> -		return -EINVAL;
> -
>  	input = kzalloc(sizeof(*input), GFP_KERNEL);
>  	if (!input)
>  		return -ENOMEM;
> @@ -2945,6 +2958,20 @@ static int igb_add_ethtool_nfc_entry(struct
> igb_adapter *adapter,
>  		input->filter.match_flags = IGB_FILTER_FLAG_ETHER_TYPE;
>  	}
> 
> +	/* Only support matching addresses by the full mask */
> +	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_source)) {
> +		input->filter.match_flags |=
> IGB_FILTER_FLAG_SRC_MAC_ADDR;
> +		ether_addr_copy(input->filter.src_addr,
> +				fsp->h_u.ether_spec.h_source);
> +	}
> +
> +	/* Only support matching addresses by the full mask */
> +	if (is_broadcast_ether_addr(fsp->m_u.ether_spec.h_dest)) {
> +		input->filter.match_flags |=
> IGB_FILTER_FLAG_DST_MAC_ADDR;
> +		ether_addr_copy(input->filter.dst_addr,
> +				fsp->h_u.ether_spec.h_dest);
> +	}
> +
>  	if ((fsp->flow_type & FLOW_EXT) && fsp->m_ext.vlan_tci) {
>  		if (fsp->m_ext.vlan_tci != htons(VLAN_PRIO_MASK)) {
>  			err = -EINVAL;
> --
> 2.16.2
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* RE: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
  2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
@ 2018-03-14  3:07     ` Brown, Aaron F
  -1 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-14  3:07 UTC (permalink / raw)
  To: Gomes, Vinicius, intel-wired-lan; +Cc: netdev, Sanchez-Palencia, Jesus

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Vinicius Costa Gomes
> Sent: Wednesday, March 7, 2018 4:37 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia@intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection
> on MAC filters on i210 and i211
> 
> On the RAH registers there are semantic differences on the meaning of
> the "queue" parameter for traffic steering depending on the controller
> model: there is the 82575 meaning, which "queue" means a RX Hardware
> Queue, and the i350 meaning, where it is a reception pool.
> 
> The previous behaviour was having no effect for i210 and i211 based
> controllers because the QSEL bit of the RAH register wasn't being set.
> 
> This patch separates the condition in discrete cases, so the different
> handling is clearer.
> 
> Fixes: 83c21335c876 ("igb: improve MAC filter handling")
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
>  drivers/net/ethernet/intel/igb/igb_main.c      | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h
> b/drivers/net/ethernet/intel/igb/e1000_defines.h
> index 83cabff1e0ab..573bf177fd08 100644
> --- a/drivers/net/ethernet/intel/igb/e1000_defines.h
> +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
> @@ -490,6 +490,7 @@
>   * manageability enabled, allowing us room for 15 multicast addresses.
>   */
>  #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
> +#define E1000_RAH_QSEL_ENABLE 0x10000000
>  #define E1000_RAL_MAC_ADDR_LEN 4
>  #define E1000_RAH_MAC_ADDR_LEN 2
>  #define E1000_RAH_POOL_MASK 0x03FC0000
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index 715bb32e6901..eabedc6b6518 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
> *adapter, u32 index)
>  		if (is_valid_ether_addr(addr))
>  			rar_high |= E1000_RAH_AV;
> 
> -		if (hw->mac.type == e1000_82575)
> +		switch (hw->mac.type) {
> +		case e1000_82575:
> +		case e1000_i210:
> +		case e1000_i211:
> +			rar_high |= E1000_RAH_QSEL_ENABLE;
>  			rar_high |= E1000_RAH_POOL_1 *
> -				    adapter->mac_table[index].queue;
> -		else
> +				      adapter->mac_table[index].queue;
> +			break;
> +		default:
>  			rar_high |= E1000_RAH_POOL_1 <<
> -				    adapter->mac_table[index].queue;
> +				adapter->mac_table[index].queue;

Small nit.  Shouldn't this line be indented more to be a few spaces past the "|=" operator as above?

> +			break;
> +		}
>  	}
> 
>  	wr32(E1000_RAL(index), rar_low);
> --
> 2.16.2
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
@ 2018-03-14  3:07     ` Brown, Aaron F
  0 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-14  3:07 UTC (permalink / raw)
  To: intel-wired-lan

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Vinicius Costa Gomes
> Sent: Wednesday, March 7, 2018 4:37 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia at intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection
> on MAC filters on i210 and i211
> 
> On the RAH registers there are semantic differences on the meaning of
> the "queue" parameter for traffic steering depending on the controller
> model: there is the 82575 meaning, which "queue" means a RX Hardware
> Queue, and the i350 meaning, where it is a reception pool.
> 
> The previous behaviour was having no effect for i210 and i211 based
> controllers because the QSEL bit of the RAH register wasn't being set.
> 
> This patch separates the condition in discrete cases, so the different
> handling is clearer.
> 
> Fixes: 83c21335c876 ("igb: improve MAC filter handling")
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
>  drivers/net/ethernet/intel/igb/igb_main.c      | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h
> b/drivers/net/ethernet/intel/igb/e1000_defines.h
> index 83cabff1e0ab..573bf177fd08 100644
> --- a/drivers/net/ethernet/intel/igb/e1000_defines.h
> +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
> @@ -490,6 +490,7 @@
>   * manageability enabled, allowing us room for 15 multicast addresses.
>   */
>  #define E1000_RAH_AV  0x80000000        /* Receive descriptor valid */
> +#define E1000_RAH_QSEL_ENABLE 0x10000000
>  #define E1000_RAL_MAC_ADDR_LEN 4
>  #define E1000_RAH_MAC_ADDR_LEN 2
>  #define E1000_RAH_POOL_MASK 0x03FC0000
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index 715bb32e6901..eabedc6b6518 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
> *adapter, u32 index)
>  		if (is_valid_ether_addr(addr))
>  			rar_high |= E1000_RAH_AV;
> 
> -		if (hw->mac.type == e1000_82575)
> +		switch (hw->mac.type) {
> +		case e1000_82575:
> +		case e1000_i210:
> +		case e1000_i211:
> +			rar_high |= E1000_RAH_QSEL_ENABLE;
>  			rar_high |= E1000_RAH_POOL_1 *
> -				    adapter->mac_table[index].queue;
> -		else
> +				      adapter->mac_table[index].queue;
> +			break;
> +		default:
>  			rar_high |= E1000_RAH_POOL_1 <<
> -				    adapter->mac_table[index].queue;
> +				adapter->mac_table[index].queue;

Small nit.  Shouldn't this line be indented more to be a few spaces past the "|=" operator as above?

> +			break;
> +		}
>  	}
> 
>  	wr32(E1000_RAL(index), rar_low);
> --
> 2.16.2
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* RE: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
  2018-03-14  3:07     ` Brown, Aaron F
@ 2018-03-14 17:25       ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-14 17:25 UTC (permalink / raw)
  To: Brown, Aaron F, intel-wired-lan; +Cc: netdev, Sanchez-Palencia, Jesus

Hi,

"Brown, Aaron F" <aaron.f.brown@intel.com> writes:

>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
>> *adapter, u32 index)
>>  		if (is_valid_ether_addr(addr))
>>  			rar_high |= E1000_RAH_AV;
>> 
>> -		if (hw->mac.type == e1000_82575)
>> +		switch (hw->mac.type) {
>> +		case e1000_82575:
>> +		case e1000_i210:
>> +		case e1000_i211:
>> +			rar_high |= E1000_RAH_QSEL_ENABLE;
>>  			rar_high |= E1000_RAH_POOL_1 *
>> -				    adapter->mac_table[index].queue;
>> -		else
>> +				      adapter->mac_table[index].queue;
>> +			break;
>> +		default:
>>  			rar_high |= E1000_RAH_POOL_1 <<
>> -				    adapter->mac_table[index].queue;
>> +				adapter->mac_table[index].queue;
>
> Small nit.  Shouldn't this line be indented more to be a few spaces
> past the "|=" operator as above?

I don't know why my editor seemed to disagree, I will fix.


Thank you,
--
Vinicius

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

* [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211
@ 2018-03-14 17:25       ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-14 17:25 UTC (permalink / raw)
  To: intel-wired-lan

Hi,

"Brown, Aaron F" <aaron.f.brown@intel.com> writes:

>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
>> *adapter, u32 index)
>>  		if (is_valid_ether_addr(addr))
>>  			rar_high |= E1000_RAH_AV;
>> 
>> -		if (hw->mac.type == e1000_82575)
>> +		switch (hw->mac.type) {
>> +		case e1000_82575:
>> +		case e1000_i210:
>> +		case e1000_i211:
>> +			rar_high |= E1000_RAH_QSEL_ENABLE;
>>  			rar_high |= E1000_RAH_POOL_1 *
>> -				    adapter->mac_table[index].queue;
>> -		else
>> +				      adapter->mac_table[index].queue;
>> +			break;
>> +		default:
>>  			rar_high |= E1000_RAH_POOL_1 <<
>> -				    adapter->mac_table[index].queue;
>> +				adapter->mac_table[index].queue;
>
> Small nit.  Shouldn't this line be indented more to be a few spaces
> past the "|=" operator as above?

I don't know why my editor seemed to disagree, I will fix.


Thank you,
--
Vinicius

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

* RE: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-14  3:04     ` Brown, Aaron F
@ 2018-03-14 19:58       ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-14 19:58 UTC (permalink / raw)
  To: Brown, Aaron F, intel-wired-lan; +Cc: netdev, Sanchez-Palencia, Jesus

Hi,

"Brown, Aaron F" <aaron.f.brown@intel.com> writes:

>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
>> Behalf Of Vinicius Costa Gomes
>> Sent: Wednesday, March 7, 2018 4:37 PM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>> palencia@intel.com>
>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>> support for ethtool nftuple filters
>> 
>> This adds the capability of configuring the queue steering of arriving
>> packets based on their source and destination MAC addresses.
>> 
>> In practical terms this adds support for the following use cases,
>> characterized by these examples:
>> 
>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>> to the RX queue 0)
>> 
>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>> (this will direct packets with source address "44:44:44:44:44:44" to
>> the RX queue 3)
>
> This seems to work fine on i210, and the patch series allows me to set
> the rx filters on the i350, i354 and i211, but it is not directing the
> packets to the queue I request.
>

For the i211, it seems that the datasheet is slightly misleading: it has
the QSEL bit documented on the RAH registers, but the queue selection
bits are not mentioned, so it really seems that queue selection won't
work for this controller.

For the other cases (in a quick search I couldn't find the i354
datasheet), the semantics changes, it's more about pool selection than
queue selection, and it depends on vfs_allocated_count (>= 1) and the
number of rss_queues (<= 1) to get to the state where setting the queue
via filters would have the expected effect.

> With the exception of i210 the rx_queues number does not seem to be
> effected by setting the filter. In the case of i211 the rx packets
> stay on rx_queue 0 with or without an ether src or dst filter. The
> first example one seems to work at first since it's directing to queue
> 0, but changing the filter to "action 1" does not change the behavior.
> With the i350 and i354 ports the packets are spread across the
> rx_queues with or without the filter set.
>

So, what I am thinking is: make it an error selecting any queue
different than zero (this is expected to work for all controllers, and
it's what will be called when the user does something like 'ip maddr'),
for controller different than the i210. Later, if/when this feature is
needed for other controllers we can extend the checks.

Does this make sense?


Thanks,
--
Vinicius

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-14 19:58       ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-14 19:58 UTC (permalink / raw)
  To: intel-wired-lan

Hi,

"Brown, Aaron F" <aaron.f.brown@intel.com> writes:

>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>> Behalf Of Vinicius Costa Gomes
>> Sent: Wednesday, March 7, 2018 4:37 PM
>> To: intel-wired-lan at lists.osuosl.org
>> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>> palencia at intel.com>
>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>> support for ethtool nftuple filters
>> 
>> This adds the capability of configuring the queue steering of arriving
>> packets based on their source and destination MAC addresses.
>> 
>> In practical terms this adds support for the following use cases,
>> characterized by these examples:
>> 
>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>> to the RX queue 0)
>> 
>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>> (this will direct packets with source address "44:44:44:44:44:44" to
>> the RX queue 3)
>
> This seems to work fine on i210, and the patch series allows me to set
> the rx filters on the i350, i354 and i211, but it is not directing the
> packets to the queue I request.
>

For the i211, it seems that the datasheet is slightly misleading: it has
the QSEL bit documented on the RAH registers, but the queue selection
bits are not mentioned, so it really seems that queue selection won't
work for this controller.

For the other cases (in a quick search I couldn't find the i354
datasheet), the semantics changes, it's more about pool selection than
queue selection, and it depends on vfs_allocated_count (>= 1) and the
number of rss_queues (<= 1) to get to the state where setting the queue
via filters would have the expected effect.

> With the exception of i210 the rx_queues number does not seem to be
> effected by setting the filter. In the case of i211 the rx packets
> stay on rx_queue 0 with or without an ether src or dst filter. The
> first example one seems to work at first since it's directing to queue
> 0, but changing the filter to "action 1" does not change the behavior.
> With the i350 and i354 ports the packets are spread across the
> rx_queues with or without the filter set.
>

So, what I am thinking is: make it an error selecting any queue
different than zero (this is expected to work for all controllers, and
it's what will be called when the user does something like 'ip maddr'),
for controller different than the i210. Later, if/when this feature is
needed for other controllers we can extend the checks.

Does this make sense?


Thanks,
--
Vinicius

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

* Re: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-14  3:04     ` Brown, Aaron F
@ 2018-03-16 17:38       ` Alexander Duyck
  -1 siblings, 0 replies; 34+ messages in thread
From: Alexander Duyck @ 2018-03-16 17:38 UTC (permalink / raw)
  To: Brown, Aaron F
  Cc: Gomes, Vinicius, intel-wired-lan, netdev, Sanchez-Palencia, Jesus

On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
>> Behalf Of Vinicius Costa Gomes
>> Sent: Wednesday, March 7, 2018 4:37 PM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>> palencia@intel.com>
>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>> support for ethtool nftuple filters
>>
>> This adds the capability of configuring the queue steering of arriving
>> packets based on their source and destination MAC addresses.
>>
>> In practical terms this adds support for the following use cases,
>> characterized by these examples:
>>
>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>> to the RX queue 0)
>>
>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>> (this will direct packets with source address "44:44:44:44:44:44" to
>> the RX queue 3)
>
> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>
> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.

Do any of the other parts actually support this functionality? I don't
think they do.

What we might look at doing instead of trying to add support for other
parts would be to explicitly limit this functionality to the i210
since if I am not mistaken this may be a feature only available in
that hardware.

Thanks.

- Alex

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-16 17:38       ` Alexander Duyck
  0 siblings, 0 replies; 34+ messages in thread
From: Alexander Duyck @ 2018-03-16 17:38 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>> Behalf Of Vinicius Costa Gomes
>> Sent: Wednesday, March 7, 2018 4:37 PM
>> To: intel-wired-lan at lists.osuosl.org
>> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>> palencia at intel.com>
>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>> support for ethtool nftuple filters
>>
>> This adds the capability of configuring the queue steering of arriving
>> packets based on their source and destination MAC addresses.
>>
>> In practical terms this adds support for the following use cases,
>> characterized by these examples:
>>
>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>> to the RX queue 0)
>>
>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>> (this will direct packets with source address "44:44:44:44:44:44" to
>> the RX queue 3)
>
> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>
> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.

Do any of the other parts actually support this functionality? I don't
think they do.

What we might look at doing instead of trying to add support for other
parts would be to explicitly limit this functionality to the i210
since if I am not mistaken this may be a feature only available in
that hardware.

Thanks.

- Alex

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

* Re: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-16 17:38       ` Alexander Duyck
@ 2018-03-16 17:59         ` Vinicius Costa Gomes
  -1 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-16 17:59 UTC (permalink / raw)
  To: Alexander Duyck, Brown, Aaron F
  Cc: intel-wired-lan, netdev, Sanchez-Palencia, Jesus

Hi,

Alexander Duyck <alexander.duyck@gmail.com> writes:

> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
>>> Behalf Of Vinicius Costa Gomes
>>> Sent: Wednesday, March 7, 2018 4:37 PM
>>> To: intel-wired-lan@lists.osuosl.org
>>> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>>> palencia@intel.com>
>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>>> support for ethtool nftuple filters
>>>
>>> This adds the capability of configuring the queue steering of arriving
>>> packets based on their source and destination MAC addresses.
>>>
>>> In practical terms this adds support for the following use cases,
>>> characterized by these examples:
>>>
>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>>> to the RX queue 0)
>>>
>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>>> (this will direct packets with source address "44:44:44:44:44:44" to
>>> the RX queue 3)
>>
>> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>>
>> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.
>
> Do any of the other parts actually support this functionality? I don't
> think they do.

>From what I can see, the only other part that supports queue steering (by MAC
addresses) is the 82575. But as I don't have any of those handy, making
it work only for the i210 seems more reasonable, to avoid getting into
this situation again.

>
> What we might look at doing instead of trying to add support for other
> parts would be to explicitly limit this functionality to the i210
> since if I am not mistaken this may be a feature only available in
> that hardware.

Sounds good to me.

>
> Thanks.
>
> - Alex


Cheers,
--
Vinicius

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-16 17:59         ` Vinicius Costa Gomes
  0 siblings, 0 replies; 34+ messages in thread
From: Vinicius Costa Gomes @ 2018-03-16 17:59 UTC (permalink / raw)
  To: intel-wired-lan

Hi,

Alexander Duyck <alexander.duyck@gmail.com> writes:

> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>>> Behalf Of Vinicius Costa Gomes
>>> Sent: Wednesday, March 7, 2018 4:37 PM
>>> To: intel-wired-lan at lists.osuosl.org
>>> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>>> palencia at intel.com>
>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>>> support for ethtool nftuple filters
>>>
>>> This adds the capability of configuring the queue steering of arriving
>>> packets based on their source and destination MAC addresses.
>>>
>>> In practical terms this adds support for the following use cases,
>>> characterized by these examples:
>>>
>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>>> to the RX queue 0)
>>>
>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>>> (this will direct packets with source address "44:44:44:44:44:44" to
>>> the RX queue 3)
>>
>> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>>
>> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.
>
> Do any of the other parts actually support this functionality? I don't
> think they do.

From what I can see, the only other part that supports queue steering (by MAC
addresses) is the 82575. But as I don't have any of those handy, making
it work only for the i210 seems more reasonable, to avoid getting into
this situation again.

>
> What we might look at doing instead of trying to add support for other
> parts would be to explicitly limit this functionality to the i210
> since if I am not mistaken this may be a feature only available in
> that hardware.

Sounds good to me.

>
> Thanks.
>
> - Alex


Cheers,
--
Vinicius

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

* Re: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-16 17:59         ` Vinicius Costa Gomes
@ 2018-03-16 18:07           ` Alexander Duyck
  -1 siblings, 0 replies; 34+ messages in thread
From: Alexander Duyck @ 2018-03-16 18:07 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: Brown, Aaron F, intel-wired-lan, netdev, Sanchez-Palencia, Jesus

On Fri, Mar 16, 2018 at 10:59 AM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi,
>
> Alexander Duyck <alexander.duyck@gmail.com> writes:
>
>> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
>>>> Behalf Of Vinicius Costa Gomes
>>>> Sent: Wednesday, March 7, 2018 4:37 PM
>>>> To: intel-wired-lan@lists.osuosl.org
>>>> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>>>> palencia@intel.com>
>>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>>>> support for ethtool nftuple filters
>>>>
>>>> This adds the capability of configuring the queue steering of arriving
>>>> packets based on their source and destination MAC addresses.
>>>>
>>>> In practical terms this adds support for the following use cases,
>>>> characterized by these examples:
>>>>
>>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>>>> to the RX queue 0)
>>>>
>>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>>>> (this will direct packets with source address "44:44:44:44:44:44" to
>>>> the RX queue 3)
>>>
>>> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>>>
>>> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.
>>
>> Do any of the other parts actually support this functionality? I don't
>> think they do.
>
> From what I can see, the only other part that supports queue steering (by MAC
> addresses) is the 82575. But as I don't have any of those handy, making
> it work only for the i210 seems more reasonable, to avoid getting into
> this situation again.

That sounds good to me. What you might do is add a comment explaining
that this is only supported on 82575 and i210 wherever you put the
check that limits this. Then if we have time for the
development/validation efforts, or someone in the community does they
could take it upon themselves to enable and test it for 82575.

I have done similar things in the past. As long as it is clear that
the reason why we limited it to i210 is mostly because of
development/validation resources somebody else can come along and
enable it if they have the resources and time to invest in doing so.

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-16 18:07           ` Alexander Duyck
  0 siblings, 0 replies; 34+ messages in thread
From: Alexander Duyck @ 2018-03-16 18:07 UTC (permalink / raw)
  To: intel-wired-lan

On Fri, Mar 16, 2018 at 10:59 AM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi,
>
> Alexander Duyck <alexander.duyck@gmail.com> writes:
>
>> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F <aaron.f.brown@intel.com> wrote:
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>>>> Behalf Of Vinicius Costa Gomes
>>>> Sent: Wednesday, March 7, 2018 4:37 PM
>>>> To: intel-wired-lan at lists.osuosl.org
>>>> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
>>>> palencia at intel.com>
>>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address
>>>> support for ethtool nftuple filters
>>>>
>>>> This adds the capability of configuring the queue steering of arriving
>>>> packets based on their source and destination MAC addresses.
>>>>
>>>> In practical terms this adds support for the following use cases,
>>>> characterized by these examples:
>>>>
>>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
>>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
>>>> to the RX queue 0)
>>>>
>>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
>>>> (this will direct packets with source address "44:44:44:44:44:44" to
>>>> the RX queue 3)
>>>
>>> This seems to work fine on i210, and the patch series allows me to set the rx filters on the i350, i354 and i211, but it is not directing the packets to the queue I request.
>>>
>>> With the exception of i210 the rx_queues number does not seem to be effected by setting the filter.  In the case of i211 the rx packets stay on rx_queue 0 with or without an ether src or dst filter.  The first example one seems to work at first since it's directing to queue 0, but changing the filter to "action 1" does not change the behavior.  With the i350 and i354 ports the packets are spread across the rx_queues with or without the filter set.
>>
>> Do any of the other parts actually support this functionality? I don't
>> think they do.
>
> From what I can see, the only other part that supports queue steering (by MAC
> addresses) is the 82575. But as I don't have any of those handy, making
> it work only for the i210 seems more reasonable, to avoid getting into
> this situation again.

That sounds good to me. What you might do is add a comment explaining
that this is only supported on 82575 and i210 wherever you put the
check that limits this. Then if we have time for the
development/validation efforts, or someone in the community does they
could take it upon themselves to enable and test it for 82575.

I have done similar things in the past. As long as it is clear that
the reason why we limited it to i210 is mostly because of
development/validation resources somebody else can come along and
enable it if they have the resources and time to invest in doing so.

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

* RE: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
  2018-03-16 18:07           ` Alexander Duyck
@ 2018-03-16 18:14             ` Brown, Aaron F
  -1 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-16 18:14 UTC (permalink / raw)
  To: Alexander Duyck, Gomes, Vinicius
  Cc: intel-wired-lan, netdev, Sanchez-Palencia, Jesus

> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> Sent: Friday, March 16, 2018 11:07 AM
> To: Gomes, Vinicius <vinicius.gomes@intel.com>
> Cc: Brown, Aaron F <aaron.f.brown@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org; Sanchez-Palencia, Jesus
> <jesus.sanchez-palencia@intel.com>
> Subject: Re: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC
> address support for ethtool nftuple filters
> 
> On Fri, Mar 16, 2018 at 10:59 AM, Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
> > Hi,
> >
> > Alexander Duyck <alexander.duyck@gmail.com> writes:
> >
> >> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F
> <aaron.f.brown@intel.com> wrote:
> >>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> >>>> Behalf Of Vinicius Costa Gomes
> >>>> Sent: Wednesday, March 7, 2018 4:37 PM
> >>>> To: intel-wired-lan@lists.osuosl.org
> >>>> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> >>>> palencia@intel.com>
> >>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC
> address
> >>>> support for ethtool nftuple filters
> >>>>
> >>>> This adds the capability of configuring the queue steering of arriving
> >>>> packets based on their source and destination MAC addresses.
> >>>>
> >>>> In practical terms this adds support for the following use cases,
> >>>> characterized by these examples:
> >>>>
> >>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
> >>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
> >>>> to the RX queue 0)
> >>>>
> >>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
> >>>> (this will direct packets with source address "44:44:44:44:44:44" to
> >>>> the RX queue 3)
> >>>
> >>> This seems to work fine on i210, and the patch series allows me to set
> the rx filters on the i350, i354 and i211, but it is not directing the packets to
> the queue I request.
> >>>
> >>> With the exception of i210 the rx_queues number does not seem to be
> effected by setting the filter.  In the case of i211 the rx packets stay on
> rx_queue 0 with or without an ether src or dst filter.  The first example one
> seems to work at first since it's directing to queue 0, but changing the filter to
> "action 1" does not change the behavior.  With the i350 and i354 ports the
> packets are spread across the rx_queues with or without the filter set.
> >>
> >> Do any of the other parts actually support this functionality? I don't
> >> think they do.
> >
> > From what I can see, the only other part that supports queue steering (by
> MAC
> > addresses) is the 82575. But as I don't have any of those handy, making
> > it work only for the i210 seems more reasonable, to avoid getting into
> > this situation again.
> 
> That sounds good to me. What you might do is add a comment explaining
> that this is only supported on 82575 and i210 wherever you put the
> check that limits this. Then if we have time for the
> development/validation efforts, or someone in the community does they
> could take it upon themselves to enable and test it for 82575.
> 
> I have done similar things in the past. As long as it is clear that
> the reason why we limited it to i210 is mostly because of
> development/validation resources somebody else can come along and
> enable it if they have the resources and time to invest in doing so.

I do have a few of 82575 NICs in my lab and am perfectly willing to test them on whatever you come up with.  But I don't believe it is a common part at all so have no qualms with limiting it to i210.

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

* [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters
@ 2018-03-16 18:14             ` Brown, Aaron F
  0 siblings, 0 replies; 34+ messages in thread
From: Brown, Aaron F @ 2018-03-16 18:14 UTC (permalink / raw)
  To: intel-wired-lan

> From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
> Sent: Friday, March 16, 2018 11:07 AM
> To: Gomes, Vinicius <vinicius.gomes@intel.com>
> Cc: Brown, Aaron F <aaron.f.brown@intel.com>; intel-wired-
> lan at lists.osuosl.org; netdev at vger.kernel.org; Sanchez-Palencia, Jesus
> <jesus.sanchez-palencia@intel.com>
> Subject: Re: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC
> address support for ethtool nftuple filters
> 
> On Fri, Mar 16, 2018 at 10:59 AM, Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
> > Hi,
> >
> > Alexander Duyck <alexander.duyck@gmail.com> writes:
> >
> >> On Tue, Mar 13, 2018 at 8:04 PM, Brown, Aaron F
> <aaron.f.brown@intel.com> wrote:
> >>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> >>>> Behalf Of Vinicius Costa Gomes
> >>>> Sent: Wednesday, March 7, 2018 4:37 PM
> >>>> To: intel-wired-lan at lists.osuosl.org
> >>>> Cc: netdev at vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> >>>> palencia at intel.com>
> >>>> Subject: [Intel-wired-lan] [next-queue PATCH v4 6/8] igb: Add MAC
> address
> >>>> support for ethtool nftuple filters
> >>>>
> >>>> This adds the capability of configuring the queue steering of arriving
> >>>> packets based on their source and destination MAC addresses.
> >>>>
> >>>> In practical terms this adds support for the following use cases,
> >>>> characterized by these examples:
> >>>>
> >>>> $ ethtool -N eth0 flow-type ether dst aa:aa:aa:aa:aa:aa action 0
> >>>> (this will direct packets with destination address "aa:aa:aa:aa:aa:aa"
> >>>> to the RX queue 0)
> >>>>
> >>>> $ ethtool -N eth0 flow-type ether src 44:44:44:44:44:44 action 3
> >>>> (this will direct packets with source address "44:44:44:44:44:44" to
> >>>> the RX queue 3)
> >>>
> >>> This seems to work fine on i210, and the patch series allows me to set
> the rx filters on the i350, i354 and i211, but it is not directing the packets to
> the queue I request.
> >>>
> >>> With the exception of i210 the rx_queues number does not seem to be
> effected by setting the filter.  In the case of i211 the rx packets stay on
> rx_queue 0 with or without an ether src or dst filter.  The first example one
> seems to work at first since it's directing to queue 0, but changing the filter to
> "action 1" does not change the behavior.  With the i350 and i354 ports the
> packets are spread across the rx_queues with or without the filter set.
> >>
> >> Do any of the other parts actually support this functionality? I don't
> >> think they do.
> >
> > From what I can see, the only other part that supports queue steering (by
> MAC
> > addresses) is the 82575. But as I don't have any of those handy, making
> > it work only for the i210 seems more reasonable, to avoid getting into
> > this situation again.
> 
> That sounds good to me. What you might do is add a comment explaining
> that this is only supported on 82575 and i210 wherever you put the
> check that limits this. Then if we have time for the
> development/validation efforts, or someone in the community does they
> could take it upon themselves to enable and test it for 82575.
> 
> I have done similar things in the past. As long as it is clear that
> the reason why we limited it to i210 is mostly because of
> development/validation resources somebody else can come along and
> enable it if they have the resources and time to invest in doing so.

I do have a few of 82575 NICs in my lab and am perfectly willing to test them on whatever you come up with.  But I don't believe it is a common part at all so have no qualms with limiting it to i210.

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

end of thread, other threads:[~2018-03-16 18:14 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-08  0:37 [next-queue PATCH v3 0/8] igb: offloading of receive filters Vinicius Costa Gomes
2018-03-08  0:37 ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 1/8] igb: Fix not adding filter elements to the list Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211 Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-14  3:07   ` Brown, Aaron F
2018-03-14  3:07     ` Brown, Aaron F
2018-03-14 17:25     ` Vinicius Costa Gomes
2018-03-14 17:25       ` Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 3/8] igb: Enable the hardware traffic class feature bit for igb models Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 4/8] igb: Add support for MAC address filters specifying source addresses Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 5/8] igb: Enable nfc filters to specify MAC addresses Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 6/8] igb: Add MAC address support for ethtool nftuple filters Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-14  3:04   ` Brown, Aaron F
2018-03-14  3:04     ` Brown, Aaron F
2018-03-14 19:58     ` Vinicius Costa Gomes
2018-03-14 19:58       ` Vinicius Costa Gomes
2018-03-16 17:38     ` Alexander Duyck
2018-03-16 17:38       ` Alexander Duyck
2018-03-16 17:59       ` Vinicius Costa Gomes
2018-03-16 17:59         ` Vinicius Costa Gomes
2018-03-16 18:07         ` Alexander Duyck
2018-03-16 18:07           ` Alexander Duyck
2018-03-16 18:14           ` Brown, Aaron F
2018-03-16 18:14             ` Brown, Aaron F
2018-03-08  0:37 ` [next-queue PATCH v4 7/8] igb: Add the skeletons for tc-flower offloading Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-03-08  0:37 ` [next-queue PATCH v4 8/8] igb: Add support for adding offloaded clsflower filters Vinicius Costa Gomes
2018-03-08  0:37   ` [Intel-wired-lan] " Vinicius Costa Gomes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.