netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support
@ 2025-06-11 18:03 Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 1/7] igc: move TXDCTL and RXDCTL related macros Tony Nguyen
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Tony Nguyen, faizal.abdul.rahim, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy

Faizal Rahim says:

MAC Merge support for frame preemption was previously added for igc:
https://lore.kernel.org/netdev/20250418163822.3519810-1-anthony.l.nguyen@intel.com/

This series builds on that work and adds support for:
- Harmonizing taprio and mqprio queue priority behavior, based on past
  discussions and suggestions:
  https://lore.kernel.org/all/20250214102206.25dqgut5tbak2rkz@skbuf/
- Enabling preemptible queue support for both taprio and mqprio, with
  priority harmonization as a prerequisite.

Patch organization:
- Patches 1-3: Preparation work for patches 6 and 7
- Patches 4-5: Queue priority harmonization
- Patches 6-7: Add preemptible queue support
---
IWL: https://lore.kernel.org/intel-wired-lan/20250519071911.2748406-1-faizal.abdul.rahim@intel.com/

The following are changes since commit 0097c4195b1d0ca57d15979626c769c74747b5a0:
  net: airoha: Add PPPoE offload support
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 1GbE

Faizal Rahim (7):
  igc: move TXDCTL and RXDCTL related macros
  igc: add DCTL prefix to related macros
  igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK
  igc: assign highest TX queue number as highest priority in mqprio
  igc: add private flag to reverse TX queue priority in TSN mode
  igc: add preemptible queue support in taprio
  igc: add preemptible queue support in mqprio

 drivers/net/ethernet/intel/igc/igc.h         |  33 +++++-
 drivers/net/ethernet/intel/igc/igc_base.h    |   8 --
 drivers/net/ethernet/intel/igc/igc_defines.h |   1 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  12 +-
 drivers/net/ethernet/intel/igc/igc_main.c    |  57 ++++++---
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 116 ++++++++++++++++---
 drivers/net/ethernet/intel/igc/igc_tsn.h     |   5 +
 7 files changed, 189 insertions(+), 43 deletions(-)

-- 
2.47.1


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

* [PATCH net-next 1/7] igc: move TXDCTL and RXDCTL related macros
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 2/7] igc: add DCTL prefix to " Tony Nguyen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

Move and consolidate TXDCTL and RXDCTL macros in preparation for
upcoming TXDCTL changes. This improves organization and readability.

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      | 11 ++++++++++-
 drivers/net/ethernet/intel/igc/igc_base.h |  8 --------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 859a15e4ccba..25695eada563 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -487,10 +487,19 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc)
  */
 #define IGC_RX_PTHRESH			8
 #define IGC_RX_HTHRESH			8
+#define IGC_RX_WTHRESH			4
+/* Ena specific Rx Queue */
+#define IGC_RXDCTL_QUEUE_ENABLE		0x02000000
+/* Receive Software Flush */
+#define IGC_RXDCTL_SWFLUSH		0x04000000
+
 #define IGC_TX_PTHRESH			8
 #define IGC_TX_HTHRESH			1
-#define IGC_RX_WTHRESH			4
 #define IGC_TX_WTHRESH			16
+/* Ena specific Tx Queue */
+#define IGC_TXDCTL_QUEUE_ENABLE		0x02000000
+/* Transmit Software Flush */
+#define IGC_TXDCTL_SWFLUSH		0x04000000
 
 #define IGC_RX_DMA_ATTR \
 	(DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING)
diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h
index 6320eabb72fe..eaf17cd031c3 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.h
+++ b/drivers/net/ethernet/intel/igc/igc_base.h
@@ -86,14 +86,6 @@ union igc_adv_rx_desc {
 	} wb;  /* writeback */
 };
 
-/* Additional Transmit Descriptor Control definitions */
-#define IGC_TXDCTL_QUEUE_ENABLE	0x02000000 /* Ena specific Tx Queue */
-#define IGC_TXDCTL_SWFLUSH	0x04000000 /* Transmit Software Flush */
-
-/* Additional Receive Descriptor Control definitions */
-#define IGC_RXDCTL_QUEUE_ENABLE	0x02000000 /* Ena specific Rx Queue */
-#define IGC_RXDCTL_SWFLUSH		0x04000000 /* Receive Software Flush */
-
 /* SRRCTL bit definitions */
 #define IGC_SRRCTL_BSIZEPKT_MASK	GENMASK(6, 0)
 #define IGC_SRRCTL_BSIZEPKT(x)		FIELD_PREP(IGC_SRRCTL_BSIZEPKT_MASK, \
-- 
2.47.1


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

* [PATCH net-next 2/7] igc: add DCTL prefix to related macros
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 1/7] igc: move TXDCTL and RXDCTL related macros Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 3/7] igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK Tony Nguyen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

Rename macros to use the DCTL prefix for consistency with existing
macros that reference the same register. This prepares for an upcoming
patch that adds new fields to TXDCTL.

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      | 12 ++++++------
 drivers/net/ethernet/intel/igc/igc_main.c | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 25695eada563..db1e2db1619e 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -485,17 +485,17 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc)
  *           descriptors until either it has this many to write back, or the
  *           ITR timer expires.
  */
-#define IGC_RX_PTHRESH			8
-#define IGC_RX_HTHRESH			8
-#define IGC_RX_WTHRESH			4
+#define IGC_RXDCTL_PTHRESH		8
+#define IGC_RXDCTL_HTHRESH		8
+#define IGC_RXDCTL_WTHRESH		4
 /* Ena specific Rx Queue */
 #define IGC_RXDCTL_QUEUE_ENABLE		0x02000000
 /* Receive Software Flush */
 #define IGC_RXDCTL_SWFLUSH		0x04000000
 
-#define IGC_TX_PTHRESH			8
-#define IGC_TX_HTHRESH			1
-#define IGC_TX_WTHRESH			16
+#define IGC_TXDCTL_PTHRESH		8
+#define IGC_TXDCTL_HTHRESH		1
+#define IGC_TXDCTL_WTHRESH		16
 /* Ena specific Tx Queue */
 #define IGC_TXDCTL_QUEUE_ENABLE		0x02000000
 /* Transmit Software Flush */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 27575a1e1777..4f1a8bc006c6 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -683,9 +683,9 @@ static void igc_configure_rx_ring(struct igc_adapter *adapter,
 
 	wr32(IGC_SRRCTL(reg_idx), srrctl);
 
-	rxdctl |= IGC_RX_PTHRESH;
-	rxdctl |= IGC_RX_HTHRESH << 8;
-	rxdctl |= IGC_RX_WTHRESH << 16;
+	rxdctl |= IGC_RXDCTL_PTHRESH;
+	rxdctl |= IGC_RXDCTL_HTHRESH << 8;
+	rxdctl |= IGC_RXDCTL_WTHRESH << 16;
 
 	/* initialize rx_buffer_info */
 	memset(ring->rx_buffer_info, 0,
@@ -749,9 +749,9 @@ static void igc_configure_tx_ring(struct igc_adapter *adapter,
 	wr32(IGC_TDH(reg_idx), 0);
 	writel(0, ring->tail);
 
-	txdctl |= IGC_TX_PTHRESH;
-	txdctl |= IGC_TX_HTHRESH << 8;
-	txdctl |= IGC_TX_WTHRESH << 16;
+	txdctl |= IGC_TXDCTL_PTHRESH;
+	txdctl |= IGC_TXDCTL_HTHRESH << 8;
+	txdctl |= IGC_TXDCTL_WTHRESH << 16;
 
 	txdctl |= IGC_TXDCTL_QUEUE_ENABLE;
 	wr32(IGC_TXDCTL(reg_idx), txdctl);
-- 
2.47.1


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

* [PATCH net-next 3/7] igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 1/7] igc: move TXDCTL and RXDCTL related macros Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 2/7] igc: add DCTL prefix to " Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 4/7] igc: assign highest TX queue number as highest priority in mqprio Tony Nguyen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

Refactor TXDCTL macro handling to use FIELD_PREP and GENMASK macros.
This prepares the code for adding a new TXDCTL priority field in an
upcoming patch.

Verified that the macro values remain unchanged before and after
refactoring.

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      | 15 ++++++++++-----
 drivers/net/ethernet/intel/igc/igc_main.c |  6 ++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index db1e2db1619e..daab06fc3f80 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -493,13 +493,18 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc)
 /* Receive Software Flush */
 #define IGC_RXDCTL_SWFLUSH		0x04000000
 
-#define IGC_TXDCTL_PTHRESH		8
-#define IGC_TXDCTL_HTHRESH		1
-#define IGC_TXDCTL_WTHRESH		16
+#define IGC_TXDCTL_PTHRESH_MASK		GENMASK(4, 0)
+#define IGC_TXDCTL_HTHRESH_MASK		GENMASK(12, 8)
+#define IGC_TXDCTL_WTHRESH_MASK		GENMASK(20, 16)
+#define IGC_TXDCTL_QUEUE_ENABLE_MASK	GENMASK(25, 25)
+#define IGC_TXDCTL_SWFLUSH_MASK		GENMASK(26, 26)
+#define IGC_TXDCTL_PTHRESH(x)		FIELD_PREP(IGC_TXDCTL_PTHRESH_MASK, (x))
+#define IGC_TXDCTL_HTHRESH(x)		FIELD_PREP(IGC_TXDCTL_HTHRESH_MASK, (x))
+#define IGC_TXDCTL_WTHRESH(x)		FIELD_PREP(IGC_TXDCTL_WTHRESH_MASK, (x))
 /* Ena specific Tx Queue */
-#define IGC_TXDCTL_QUEUE_ENABLE		0x02000000
+#define IGC_TXDCTL_QUEUE_ENABLE		FIELD_PREP(IGC_TXDCTL_QUEUE_ENABLE_MASK, 1)
 /* Transmit Software Flush */
-#define IGC_TXDCTL_SWFLUSH		0x04000000
+#define IGC_TXDCTL_SWFLUSH		FIELD_PREP(IGC_TXDCTL_SWFLUSH_MASK, 1)
 
 #define IGC_RX_DMA_ATTR \
 	(DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 4f1a8bc006c6..f3a312c9413b 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -749,11 +749,9 @@ static void igc_configure_tx_ring(struct igc_adapter *adapter,
 	wr32(IGC_TDH(reg_idx), 0);
 	writel(0, ring->tail);
 
-	txdctl |= IGC_TXDCTL_PTHRESH;
-	txdctl |= IGC_TXDCTL_HTHRESH << 8;
-	txdctl |= IGC_TXDCTL_WTHRESH << 16;
+	txdctl |= IGC_TXDCTL_PTHRESH(8) | IGC_TXDCTL_HTHRESH(1) |
+		  IGC_TXDCTL_WTHRESH(16) | IGC_TXDCTL_QUEUE_ENABLE;
 
-	txdctl |= IGC_TXDCTL_QUEUE_ENABLE;
 	wr32(IGC_TXDCTL(reg_idx), txdctl);
 }
 
-- 
2.47.1


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

* [PATCH net-next 4/7] igc: assign highest TX queue number as highest priority in mqprio
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (2 preceding siblings ...)
  2025-06-11 18:03 ` [PATCH net-next 3/7] igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode Tony Nguyen
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

Previously, TX arbitration prioritized queues based on the TC they were
mapped to. A queue mapped to TC 3 had higher priority than one mapped to
TC 0.

To improve code reuse for upcoming patches and align with typical NIC
behavior, this patch updates the logic to prioritize higher queue numbers
when mqprio is used. As a result, queue 0 becomes the lowest priority and
queue 3 becomes the highest.

This patch also introduces igc_tsn_is_tc_to_queue_priority_ordered() to
preserve the original TC-based priority rule and reject configurations
where a higher TC maps to a lower queue offset.

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 20 +++++++++++++
 drivers/net/ethernet/intel/igc/igc_tsn.c  | 35 ++++++++++++++---------
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index f3a312c9413b..1322a2db6dba 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6724,6 +6724,20 @@ static void igc_save_mqprio_params(struct igc_adapter *adapter, u8 num_tc,
 		adapter->queue_per_tc[i] = offset[i];
 }
 
+static bool
+igc_tsn_is_tc_to_queue_priority_ordered(struct tc_mqprio_qopt_offload *mqprio)
+{
+	int num_tc = mqprio->qopt.num_tc;
+	int i;
+
+	for (i = 1; i < num_tc; i++) {
+		if (mqprio->qopt.offset[i - 1] > mqprio->qopt.offset[i])
+			return false;
+	}
+
+	return true;
+}
+
 static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 				 struct tc_mqprio_qopt_offload *mqprio)
 {
@@ -6756,6 +6770,12 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 		}
 	}
 
+	if (!igc_tsn_is_tc_to_queue_priority_ordered(mqprio)) {
+		NL_SET_ERR_MSG_MOD(mqprio->extack,
+				   "tc to queue mapping must preserve increasing priority (higher tc -> higher queue)");
+		return -EOPNOTSUPP;
+	}
+
 	/* Preemption is not supported yet. */
 	if (mqprio->preemptible_tcs) {
 		NL_SET_ERR_MSG_MOD(mqprio->extack,
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index f22cc4d4f459..78a4a9cf5f96 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -13,6 +13,13 @@
 #define TX_MAX_FRAG_SIZE	(TX_MIN_FRAG_SIZE * \
 				 (MAX_MULTPLIER_TX_MIN_FRAG + 1))
 
+enum tx_queue {
+	TX_QUEUE_0 = 0,
+	TX_QUEUE_1,
+	TX_QUEUE_2,
+	TX_QUEUE_3,
+};
+
 DEFINE_STATIC_KEY_FALSE(igc_fpe_enabled);
 
 static int igc_fpe_init_smd_frame(struct igc_ring *ring,
@@ -238,7 +245,7 @@ bool igc_tsn_is_taprio_activated_by_user(struct igc_adapter *adapter)
 		adapter->taprio_offload_enable;
 }
 
-static void igc_tsn_tx_arb(struct igc_adapter *adapter, u16 *queue_per_tc)
+static void igc_tsn_tx_arb(struct igc_adapter *adapter, bool reverse_prio)
 {
 	struct igc_hw *hw = &adapter->hw;
 	u32 txarb;
@@ -250,10 +257,17 @@ static void igc_tsn_tx_arb(struct igc_adapter *adapter, u16 *queue_per_tc)
 		   IGC_TXARB_TXQ_PRIO_2_MASK |
 		   IGC_TXARB_TXQ_PRIO_3_MASK);
 
-	txarb |= IGC_TXARB_TXQ_PRIO_0(queue_per_tc[3]);
-	txarb |= IGC_TXARB_TXQ_PRIO_1(queue_per_tc[2]);
-	txarb |= IGC_TXARB_TXQ_PRIO_2(queue_per_tc[1]);
-	txarb |= IGC_TXARB_TXQ_PRIO_3(queue_per_tc[0]);
+	if (reverse_prio) {
+		txarb |= IGC_TXARB_TXQ_PRIO_0(TX_QUEUE_3);
+		txarb |= IGC_TXARB_TXQ_PRIO_1(TX_QUEUE_2);
+		txarb |= IGC_TXARB_TXQ_PRIO_2(TX_QUEUE_1);
+		txarb |= IGC_TXARB_TXQ_PRIO_3(TX_QUEUE_0);
+	} else {
+		txarb |= IGC_TXARB_TXQ_PRIO_0(TX_QUEUE_0);
+		txarb |= IGC_TXARB_TXQ_PRIO_1(TX_QUEUE_1);
+		txarb |= IGC_TXARB_TXQ_PRIO_2(TX_QUEUE_2);
+		txarb |= IGC_TXARB_TXQ_PRIO_3(TX_QUEUE_3);
+	}
 
 	wr32(IGC_TXARB, txarb);
 }
@@ -286,7 +300,6 @@ static void igc_tsn_set_rxpbsize(struct igc_adapter *adapter,
  */
 static int igc_tsn_disable_offload(struct igc_adapter *adapter)
 {
-	u16 queue_per_tc[4] = { 3, 2, 1, 0 };
 	struct igc_hw *hw = &adapter->hw;
 	u32 tqavctrl;
 	int i;
@@ -319,7 +332,7 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
 	/* Restore the default Tx arbitration: Priority 0 has the highest
 	 * priority and is assigned to queue 0 and so on and so forth.
 	 */
-	igc_tsn_tx_arb(adapter, queue_per_tc);
+	igc_tsn_tx_arb(adapter, false);
 
 	adapter->flags &= ~IGC_FLAG_TSN_QBV_ENABLED;
 
@@ -385,12 +398,8 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 	if (igc_is_device_id_i226(hw))
 		igc_tsn_set_retx_qbvfullthreshold(adapter);
 
-	if (adapter->strict_priority_enable) {
-		/* Configure queue priorities according to the user provided
-		 * mapping.
-		 */
-		igc_tsn_tx_arb(adapter, adapter->queue_per_tc);
-	}
+	if (adapter->strict_priority_enable)
+		igc_tsn_tx_arb(adapter, true);
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
 		struct igc_ring *ring = adapter->tx_ring[i];
-- 
2.47.1


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

* [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (3 preceding siblings ...)
  2025-06-11 18:03 ` [PATCH net-next 4/7] igc: assign highest TX queue number as highest priority in mqprio Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-17 10:06   ` Paolo Abeni
  2025-06-11 18:03 ` [PATCH net-next 6/7] igc: add preemptible queue support in taprio Tony Nguyen
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

By default, igc assigns TX hw queue 0 the highest priority and queue 3
the lowest. This is opposite of most NICs, where TX hw queue 3 has the
highest priority and queue 0 the lowest.

mqprio in igc already uses TX arbitration unconditionally to reverse TX
queue priority when mqprio is enabled. The TX arbitration logic does not
require a private flag, because mqprio was added recently and no known
users depend on the default queue ordering, which differs from the typical
convention.

taprio does not use TX arbitration, so it inherits the default igc TX
queue priority order. This causes tc command inconsistencies when
configuring frame preemption with taprio compared to mqprio in igc.
Other tc command inconsistencies and configuration issues already exist
when using taprio on igc compared to other network controllers. These
issues are described in a later section.

To harmonize TX queue priority behavior between taprio and mqprio, and
to fix these issues without breaking long-standing taprio use cases,
this patch adds a new private flag, called reverse-tsn-txq-prio, to
reverse the TX queue priority. It makes queue 3 the highest and queue 0
the lowest, reusing the TX arbitration logic already used by mqprio.

Users must set the private flag when enabling frame preemption with
taprio to follow the standard convention. Doing so promotes adoption of
the correct priority model for new features while preserving
compatibility with legacy configurations.

This new private flag addresses:

1.  Non-standard socket -> tc -> TX hw queue mapping for taprio in igc

Without the private flag:
- taprio maps (socket -> tc -> TX hardware queue) differently on igc
  compared to other network controllers
- On igc, mqprio maps tc differently from taprio, since mqprio already
  uses TX arbitration

The following examples compare taprio configuration on igc and other
network controllers:
a)  On other NICs (TX hw queue 3 is highest priority):
    taprio num_tc 4 map 0 1 2 3 .... \
    queues 1@0 1@1 1@2 1@3

    Mapping translates to:
    socket 0 -> tc 0 -> queue 0
    socket 3 -> tc 3 -> queue 3

    This is the normal mapping that respects the standard convention:
    higher socket number -> higher tc -> higher priority TX hw queue

b)  On igc (TX hw queue 0 is highest priority by default):
    taprio num_tc 4 map 3 2 1 0 .... \
    queues 1@0 1@1 1@2 1@3

    Mapping translates to:
    socket 0 -> tc 3 -> queue 3
    socket 3 -> tc 0 -> queue 0

    This igc tc mapping example is based on Intel's TSN validation test
    case, where a higher socket priority maps to a higher priority queue.
    It respects the mapping:
      higher socket number -> higher priority TX hw queue
    but breaks the expected ordering:
      higher tc -> higher priority TX hw queue
    as defined in [Ref1]. This custom mapping complicates common taprio
    setup across NICs.

2.  Non-standard frame preemption mapping for taprio in igc

Without the private flag:
- Compared to other network controllers, taprio on igc must flip the
  expected fp sequence, since express traffic is expected to map to the
  highest priority queue and preemptible traffic to lower ones
- On igc, frame preemption configuration for mqprio differs from taprio,
  since mqprio already uses TX arbitration

The following examples compare taprio frame preemption configuration on
igc and other network controllers:
a)  On other NICs (TX hw queue 3 is highest priority):
    taprio num_tc 4 map ..... \
    queues 1@0 1@1 1@2 1@3 \
    fp P P P E

    Mapping translates to:
    tc0, tc1, tc2 -> preemptible -> queue 0, 1, 2
    tc3           -> express     -> queue 3

    This is the normal mapping that respects the standard convention:
    higher tc -> express traffic -> higher priority TX hw queue
    lower tc  -> preemptible traffic -> lower priority TX hw queue

b)  On igc (TX hw queue 0 is highest priority by default):
    taprio num_tc 4 map ...... \
    queues 1@0 1@1 1@2 1@3 \
    fp E P P P

    Mapping translates to:
    tc0           -> express     -> queue 0
    tc1, tc2, tc3 -> preemptible -> queue 1, 2, 3

    This inversion respects the mapping of:
      express traffic -> higher priority TX hw queue
    but breaks the expected ordering:
      higher tc -> express traffic
    as defined in [Ref1] where higher tc indicates higher priority. In
    this case, the lower tc0 is assigned to express traffic. This custom
    mapping further complicates common preemption setup across NICs.

Tests were performed on taprio with the following combinations, where
two apps send traffic simultaneously on different queues:

  Private Flag   Traffic Sent By           Traffic Sent By
  ----------------------------------------------------------------
  enabled        iperf3 (queue 3)          iperf3 (queue 0)
  disabled       iperf3 (queue 0)          iperf3 (queue 3)
  enabled        iperf3 (queue 3)          real-time app (queue 0)
  disabled       iperf3 (queue 0)          real-time app (queue 3)
  enabled        real-time app (queue 3)   iperf3 (queue 0)
  disabled       real-time app (queue 0)   iperf3 (queue 3)
  enabled        real-time app (queue 3)   real-time app (queue 0)
  disabled       real-time app (queue 0)   real-time app (queue 3)

Private flag is controlled with:
 ethtool --set-priv-flags enp1s0 reverse-tsn-txq-prio <on|off>

[Ref1]
IEEE 802.1Q clause 8.6.8 Transmission selection:
"For a given Port and traffic class, frames are selected from the
corresponding queue for transmission if and only if:
...
b) For each queue corresponding to a numerically higher value of traffic
class supported by the Port, the operation of the transmission selection
algorithm supported by that queue determines that there is no frame
available for transmission."

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h         |  1 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 12 ++++++++++--
 drivers/net/ethernet/intel/igc/igc_main.c    |  3 ++-
 drivers/net/ethernet/intel/igc/igc_tsn.c     |  3 ++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index daab06fc3f80..023ff8a5b285 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -395,6 +395,7 @@ extern char igc_driver_name[];
 #define IGC_FLAG_TSN_QBV_ENABLED	BIT(17)
 #define IGC_FLAG_TSN_QAV_ENABLED	BIT(18)
 #define IGC_FLAG_TSN_PREEMPT_ENABLED	BIT(19)
+#define IGC_FLAG_TSN_REVERSE_TXQ_PRIO	BIT(20)
 
 #define IGC_FLAG_TSN_ANY_ENABLED				\
 	(IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED |	\
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 3fc1eded9605..054b7390cb4b 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -122,9 +122,11 @@ static const char igc_gstrings_test[][ETH_GSTRING_LEN] = {
 #define IGC_STATS_LEN \
 	(IGC_GLOBAL_STATS_LEN + IGC_NETDEV_STATS_LEN + IGC_QUEUE_STATS_LEN)
 
+#define IGC_PRIV_FLAGS_LEGACY_RX		BIT(0)
+#define IGC_PRIV_FLAGS_REVERSE_TSN_TXQ_PRIO	BIT(1)
 static const char igc_priv_flags_strings[][ETH_GSTRING_LEN] = {
-#define IGC_PRIV_FLAGS_LEGACY_RX	BIT(0)
 	"legacy-rx",
+	"reverse-tsn-txq-prio",
 };
 
 #define IGC_PRIV_FLAGS_STR_LEN ARRAY_SIZE(igc_priv_flags_strings)
@@ -1600,6 +1602,9 @@ static u32 igc_ethtool_get_priv_flags(struct net_device *netdev)
 	if (adapter->flags & IGC_FLAG_RX_LEGACY)
 		priv_flags |= IGC_PRIV_FLAGS_LEGACY_RX;
 
+	if (adapter->flags & IGC_FLAG_TSN_REVERSE_TXQ_PRIO)
+		priv_flags |= IGC_PRIV_FLAGS_REVERSE_TSN_TXQ_PRIO;
+
 	return priv_flags;
 }
 
@@ -1608,10 +1613,13 @@ static int igc_ethtool_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 	struct igc_adapter *adapter = netdev_priv(netdev);
 	unsigned int flags = adapter->flags;
 
-	flags &= ~IGC_FLAG_RX_LEGACY;
+	flags &= ~(IGC_FLAG_RX_LEGACY | IGC_FLAG_TSN_REVERSE_TXQ_PRIO);
 	if (priv_flags & IGC_PRIV_FLAGS_LEGACY_RX)
 		flags |= IGC_FLAG_RX_LEGACY;
 
+	if (priv_flags & IGC_PRIV_FLAGS_REVERSE_TSN_TXQ_PRIO)
+		flags |= IGC_FLAG_TSN_REVERSE_TXQ_PRIO;
+
 	if (flags != adapter->flags) {
 		adapter->flags = flags;
 
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 1322a2db6dba..82d53fad6b6a 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6698,7 +6698,8 @@ static int igc_tc_query_caps(struct igc_adapter *adapter,
 	case TC_SETUP_QDISC_TAPRIO: {
 		struct tc_taprio_caps *caps = base->caps;
 
-		caps->broken_mqprio = true;
+		if (!(adapter->flags & IGC_FLAG_TSN_REVERSE_TXQ_PRIO))
+			caps->broken_mqprio = true;
 
 		if (hw->mac.type == igc_i225) {
 			caps->supports_queue_max_sdu = true;
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 78a4a9cf5f96..43151ab4c1b7 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -398,7 +398,8 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 	if (igc_is_device_id_i226(hw))
 		igc_tsn_set_retx_qbvfullthreshold(adapter);
 
-	if (adapter->strict_priority_enable)
+	if (adapter->strict_priority_enable ||
+	    adapter->flags & IGC_FLAG_TSN_REVERSE_TXQ_PRIO)
 		igc_tsn_tx_arb(adapter, true);
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
-- 
2.47.1


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

* [PATCH net-next 6/7] igc: add preemptible queue support in taprio
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (4 preceding siblings ...)
  2025-06-11 18:03 ` [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-11 18:03 ` [PATCH net-next 7/7] igc: add preemptible queue support in mqprio Tony Nguyen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Aleksandr Loktionov, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

Changes:
1. Introduce tx_enabled flag to control preemptible queue. tx_enabled
   is set via mmsv module based on multiple factors, including link
   up/down status, to determine if FPE is active or inactive.
2. Add priority field to TXDCTL for express queue to improve data
   fetch performance.
3. Block preemptible queue setup in taprio unless reverse-tsn-txq-prio
   private flag is set. Encourages adoption of standard queue priority
   scheme for new features.
4. Hardware-padded frames from preemptible queues result in incorrect
   mCRC values, as padding bytes are excluded from the computation. Pad
   frames to at least 60 bytes using skb_padto() before transmission to
   ensure the hardware includes padding in the mCRC calculation.

Tested preemption with taprio by:
1. Enable FPE:
   ethtool --set-mm enp1s0 pmac-enabled on tx-enabled on verify-enabled on
2. Enable private flag to reverse TX queue priority:
   ethtool --set-priv-flags enp1s0 reverse-txq-prio on
3. Enable preemptible queue in taprio:
   taprio num_tc 4 map 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \
   queues 1@0 1@1 1@2 1@3 \
   fp P P P E

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Co-developed-by: Chwee-Lin Choong <chwee.lin.choong@intel.com>
Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@intel.com>
Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h         |  6 ++
 drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c    | 21 +++++-
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 71 ++++++++++++++++++++
 drivers/net/ethernet/intel/igc/igc_tsn.h     |  4 ++
 5 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 023ff8a5b285..1525ae25fd3e 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -43,6 +43,7 @@ void igc_ethtool_set_ops(struct net_device *);
 struct igc_fpe_t {
 	struct ethtool_mmsv mmsv;
 	u32 tx_min_frag_size;
+	bool tx_enabled;
 };
 
 enum igc_mac_filter_type {
@@ -163,6 +164,7 @@ struct igc_ring {
 	bool launchtime_enable;         /* true if LaunchTime is enabled */
 	ktime_t last_tx_cycle;          /* end of the cycle with a launchtime transmission */
 	ktime_t last_ff_cycle;          /* Last cycle with an active first flag */
+	bool preemptible;		/* True if preemptible queue, false if express queue */
 
 	u32 start_time;
 	u32 end_time;
@@ -499,6 +501,8 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc)
 #define IGC_TXDCTL_WTHRESH_MASK		GENMASK(20, 16)
 #define IGC_TXDCTL_QUEUE_ENABLE_MASK	GENMASK(25, 25)
 #define IGC_TXDCTL_SWFLUSH_MASK		GENMASK(26, 26)
+#define IGC_TXDCTL_PRIORITY_MASK	GENMASK(27, 27)
+
 #define IGC_TXDCTL_PTHRESH(x)		FIELD_PREP(IGC_TXDCTL_PTHRESH_MASK, (x))
 #define IGC_TXDCTL_HTHRESH(x)		FIELD_PREP(IGC_TXDCTL_HTHRESH_MASK, (x))
 #define IGC_TXDCTL_WTHRESH(x)		FIELD_PREP(IGC_TXDCTL_WTHRESH_MASK, (x))
@@ -506,6 +510,8 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc)
 #define IGC_TXDCTL_QUEUE_ENABLE		FIELD_PREP(IGC_TXDCTL_QUEUE_ENABLE_MASK, 1)
 /* Transmit Software Flush */
 #define IGC_TXDCTL_SWFLUSH		FIELD_PREP(IGC_TXDCTL_SWFLUSH_MASK, 1)
+#define IGC_TXDCTL_PRIORITY(x)		FIELD_PREP(IGC_TXDCTL_PRIORITY_MASK, (x))
+#define IGC_TXDCTL_PRIORITY_HIGH	IGC_TXDCTL_PRIORITY(1)
 
 #define IGC_RX_DMA_ATTR \
 	(DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING)
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index 7189dfc389ad..86b346687196 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -588,6 +588,7 @@
 #define IGC_TXQCTL_QUEUE_MODE_LAUNCHT	0x00000001
 #define IGC_TXQCTL_STRICT_CYCLE		0x00000002
 #define IGC_TXQCTL_STRICT_END		0x00000004
+#define IGC_TXQCTL_PREEMPTIBLE		0x00000008
 #define IGC_TXQCTL_QAV_SEL_MASK		0x000000C0
 #define IGC_TXQCTL_QAV_SEL_CBS0		0x00000080
 #define IGC_TXQCTL_QAV_SEL_CBS1		0x000000C0
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 82d53fad6b6a..23cbe02ee238 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1685,6 +1685,15 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 	first->tx_flags = tx_flags;
 	first->protocol = protocol;
 
+	/* For preemptible queue, manually pad the skb so that HW includes
+	 * padding bytes in mCRC calculation
+	 */
+	if (tx_ring->preemptible && skb->len < ETH_ZLEN) {
+		if (skb_padto(skb, ETH_ZLEN))
+			goto out_drop;
+		skb_put(skb, ETH_ZLEN - skb->len);
+	}
+
 	tso = igc_tso(tx_ring, first, launch_time, first_flag, &hdr_len);
 	if (tso < 0)
 		goto out_drop;
@@ -6419,6 +6428,7 @@ static int igc_qbv_clear_schedule(struct igc_adapter *adapter)
 		ring->start_time = 0;
 		ring->end_time = NSEC_PER_SEC;
 		ring->max_sdu = 0;
+		ring->preemptible = false;
 	}
 
 	spin_lock_irqsave(&adapter->qbv_tx_lock, flags);
@@ -6484,9 +6494,12 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 	if (!validate_schedule(adapter, qopt))
 		return -EINVAL;
 
-	/* preemptible isn't supported yet */
-	if (qopt->mqprio.preemptible_tcs)
-		return -EOPNOTSUPP;
+	if (qopt->mqprio.preemptible_tcs &&
+	    !(adapter->flags & IGC_FLAG_TSN_REVERSE_TXQ_PRIO)) {
+		NL_SET_ERR_MSG_MOD(qopt->extack,
+				   "reverse-tsn-txq-prio private flag must be enabled before setting preemptible tc");
+		return -ENODEV;
+	}
 
 	igc_ptp_read(adapter, &now);
 
@@ -6579,6 +6592,8 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 			ring->max_sdu = 0;
 	}
 
+	igc_fpe_save_preempt_queue(adapter, &qopt->mqprio);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 43151ab4c1b7..811856d66571 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -116,6 +116,18 @@ static int igc_fpe_xmit_smd_frame(struct igc_adapter *adapter,
 	return err;
 }
 
+static void igc_fpe_configure_tx(struct ethtool_mmsv *mmsv, bool tx_enable)
+{
+	struct igc_fpe_t *fpe = container_of(mmsv, struct igc_fpe_t, mmsv);
+	struct igc_adapter *adapter;
+
+	adapter = container_of(fpe, struct igc_adapter, fpe);
+	adapter->fpe.tx_enabled = tx_enable;
+
+	/* Update config since tx_enabled affects preemptible queue configuration */
+	igc_tsn_offload_apply(adapter);
+}
+
 static void igc_fpe_send_mpacket(struct ethtool_mmsv *mmsv,
 				 enum ethtool_mpacket type)
 {
@@ -137,15 +149,50 @@ static void igc_fpe_send_mpacket(struct ethtool_mmsv *mmsv,
 }
 
 static const struct ethtool_mmsv_ops igc_mmsv_ops = {
+	.configure_tx = igc_fpe_configure_tx,
 	.send_mpacket = igc_fpe_send_mpacket,
 };
 
 void igc_fpe_init(struct igc_adapter *adapter)
 {
 	adapter->fpe.tx_min_frag_size = TX_MIN_FRAG_SIZE;
+	adapter->fpe.tx_enabled = false;
 	ethtool_mmsv_init(&adapter->fpe.mmsv, adapter->netdev, &igc_mmsv_ops);
 }
 
+static u32 igc_fpe_map_preempt_tc_to_queue(const struct igc_adapter *adapter,
+					   unsigned long preemptible_tcs)
+{
+	struct net_device *dev = adapter->netdev;
+	u32 i, queue = 0;
+
+	for (i = 0; i < dev->num_tc; i++) {
+		u32 offset, count;
+
+		if (!(preemptible_tcs & BIT(i)))
+			continue;
+
+		offset = dev->tc_to_txq[i].offset;
+		count = dev->tc_to_txq[i].count;
+		queue |= GENMASK(offset + count - 1, offset);
+	}
+
+	return queue;
+}
+
+void igc_fpe_save_preempt_queue(struct igc_adapter *adapter,
+				const struct tc_mqprio_qopt_offload *mqprio)
+{
+	u32 preemptible_queue = igc_fpe_map_preempt_tc_to_queue(adapter,
+								mqprio->preemptible_tcs);
+
+	for (int i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *tx_ring = adapter->tx_ring[i];
+
+		tx_ring->preemptible = !!(preemptible_queue & BIT(i));
+	}
+}
+
 static bool is_any_launchtime(struct igc_adapter *adapter)
 {
 	int i;
@@ -321,9 +368,16 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
 	wr32(IGC_TQAVCTRL, tqavctrl);
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
+		int reg_idx = adapter->tx_ring[i]->reg_idx;
+		u32 txdctl;
+
 		wr32(IGC_TXQCTL(i), 0);
 		wr32(IGC_STQT(i), 0);
 		wr32(IGC_ENDQT(i), NSEC_PER_SEC);
+
+		txdctl = rd32(IGC_TXDCTL(reg_idx));
+		txdctl &= ~IGC_TXDCTL_PRIORITY_HIGH;
+		wr32(IGC_TXDCTL(reg_idx), txdctl);
 	}
 
 	wr32(IGC_QBVCYCLET_S, 0);
@@ -404,6 +458,7 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
 		struct igc_ring *ring = adapter->tx_ring[i];
+		u32 txdctl = rd32(IGC_TXDCTL(ring->reg_idx));
 		u32 txqctl = 0;
 		u16 cbs_value;
 		u32 tqavcc;
@@ -437,6 +492,22 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 		if (ring->launchtime_enable)
 			txqctl |= IGC_TXQCTL_QUEUE_MODE_LAUNCHT;
 
+		if (!adapter->fpe.tx_enabled) {
+			/* fpe inactive: clear both flags */
+			txqctl &= ~IGC_TXQCTL_PREEMPTIBLE;
+			txdctl &= ~IGC_TXDCTL_PRIORITY_HIGH;
+		} else if (ring->preemptible) {
+			/* fpe active + preemptible: enable preemptible queue + set low priority */
+			txqctl |= IGC_TXQCTL_PREEMPTIBLE;
+			txdctl &= ~IGC_TXDCTL_PRIORITY_HIGH;
+		} else {
+			/* fpe active + express: enable express queue + set high priority */
+			txqctl &= ~IGC_TXQCTL_PREEMPTIBLE;
+			txdctl |= IGC_TXDCTL_PRIORITY_HIGH;
+		}
+
+		wr32(IGC_TXDCTL(ring->reg_idx), txdctl);
+
 		/* Skip configuring CBS for Q2 and Q3 */
 		if (i > 1)
 			goto skip_cbs;
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
index c2a77229207b..f2e8bfef4871 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.h
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
@@ -4,6 +4,8 @@
 #ifndef _IGC_TSN_H_
 #define _IGC_TSN_H_
 
+#include <net/pkt_sched.h>
+
 #define IGC_RX_MIN_FRAG_SIZE		60
 #define SMD_FRAME_SIZE			60
 
@@ -15,6 +17,8 @@ enum igc_txd_popts_type {
 DECLARE_STATIC_KEY_FALSE(igc_fpe_enabled);
 
 void igc_fpe_init(struct igc_adapter *adapter);
+void igc_fpe_save_preempt_queue(struct igc_adapter *adapter,
+				const struct tc_mqprio_qopt_offload *mqprio);
 u32 igc_fpe_get_supported_frag_size(u32 frag_size);
 int igc_tsn_offload_apply(struct igc_adapter *adapter);
 int igc_tsn_reset(struct igc_adapter *adapter);
-- 
2.47.1


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

* [PATCH net-next 7/7] igc: add preemptible queue support in mqprio
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (5 preceding siblings ...)
  2025-06-11 18:03 ` [PATCH net-next 6/7] igc: add preemptible queue support in taprio Tony Nguyen
@ 2025-06-11 18:03 ` Tony Nguyen
  2025-06-13  1:42 ` [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Jakub Kicinski
  2025-06-17 13:10 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Nguyen @ 2025-06-11 18:03 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
  Cc: Faizal Rahim, anthony.l.nguyen, faizal.abdul.rahim,
	chwee.lin.choong, vladimir.oltean, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay

From: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>

igc already supports enabling MAC Merge for FPE. This patch adds
support for preemptible queues in mqprio.

Tested preemption with mqprio by:
1. Enable FPE:
   ethtool --set-mm enp1s0 pmac-enabled on tx-enabled on verify-enabled on
2. Enable preemptible queue in mqprio:
   mqprio num_tc 4 map 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \
   queues 1@0 1@1 1@2 1@3 \
   fp P P P E

Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 9 ++-------
 drivers/net/ethernet/intel/igc/igc_tsn.c  | 9 +++++++++
 drivers/net/ethernet/intel/igc/igc_tsn.h  | 1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 23cbe02ee238..515b9610b907 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6765,6 +6765,7 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 
 	if (!mqprio->qopt.num_tc) {
 		adapter->strict_priority_enable = false;
+		igc_fpe_clear_preempt_queue(adapter);
 		netdev_reset_tc(adapter->netdev);
 		goto apply;
 	}
@@ -6792,13 +6793,6 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 		return -EOPNOTSUPP;
 	}
 
-	/* Preemption is not supported yet. */
-	if (mqprio->preemptible_tcs) {
-		NL_SET_ERR_MSG_MOD(mqprio->extack,
-				   "Preemption is not supported yet");
-		return -EOPNOTSUPP;
-	}
-
 	igc_save_mqprio_params(adapter, mqprio->qopt.num_tc,
 			       mqprio->qopt.offset);
 
@@ -6818,6 +6812,7 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 		adapter->queue_per_tc[i] = i;
 
 	mqprio->qopt.hw = TC_MQPRIO_HW_OFFLOAD_TCS;
+	igc_fpe_save_preempt_queue(adapter, mqprio);
 
 apply:
 	return igc_tsn_offload_apply(adapter);
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 811856d66571..b23b9ca451a7 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -160,6 +160,15 @@ void igc_fpe_init(struct igc_adapter *adapter)
 	ethtool_mmsv_init(&adapter->fpe.mmsv, adapter->netdev, &igc_mmsv_ops);
 }
 
+void igc_fpe_clear_preempt_queue(struct igc_adapter *adapter)
+{
+	for (int i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *tx_ring = adapter->tx_ring[i];
+
+		tx_ring->preemptible = false;
+	}
+}
+
 static u32 igc_fpe_map_preempt_tc_to_queue(const struct igc_adapter *adapter,
 					   unsigned long preemptible_tcs)
 {
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
index f2e8bfef4871..a95b893459d7 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.h
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
@@ -17,6 +17,7 @@ enum igc_txd_popts_type {
 DECLARE_STATIC_KEY_FALSE(igc_fpe_enabled);
 
 void igc_fpe_init(struct igc_adapter *adapter);
+void igc_fpe_clear_preempt_queue(struct igc_adapter *adapter);
 void igc_fpe_save_preempt_queue(struct igc_adapter *adapter,
 				const struct tc_mqprio_qopt_offload *mqprio);
 u32 igc_fpe_get_supported_frag_size(u32 frag_size);
-- 
2.47.1


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

* Re: [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (6 preceding siblings ...)
  2025-06-11 18:03 ` [PATCH net-next 7/7] igc: add preemptible queue support in mqprio Tony Nguyen
@ 2025-06-13  1:42 ` Jakub Kicinski
  2025-06-17 13:10 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2025-06-13  1:42 UTC (permalink / raw)
  To: Tony Nguyen
  Cc: davem, pabeni, edumazet, andrew+netdev, netdev,
	faizal.abdul.rahim, faizal.abdul.rahim, chwee.lin.choong,
	vladimir.oltean, horms, vitaly.lifshits, dima.ruinskiy

On Wed, 11 Jun 2025 11:03:02 -0700 Tony Nguyen wrote:
> MAC Merge support for frame preemption was previously added for igc:
> https://lore.kernel.org/netdev/20250418163822.3519810-1-anthony.l.nguyen@intel.com/
> 
> This series builds on that work and adds support for:
> - Harmonizing taprio and mqprio queue priority behavior, based on past
>   discussions and suggestions:
>   https://lore.kernel.org/all/20250214102206.25dqgut5tbak2rkz@skbuf/
> - Enabling preemptible queue support for both taprio and mqprio, with
>   priority harmonization as a prerequisite.

I'd like to hold these in patchwork for a little longer, in case
Vladimir finds the time to take a look. 
So feel free to send another series for net-next while we wait.

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

* Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-11 18:03 ` [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode Tony Nguyen
@ 2025-06-17 10:06   ` Paolo Abeni
  2025-06-17 12:17     ` Vladimir Oltean
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2025-06-17 10:06 UTC (permalink / raw)
  To: Tony Nguyen, Faizal Rahim, vladimir.oltean
  Cc: faizal.abdul.rahim, chwee.lin.choong, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay, davem, edumazet, andrew+netdev,
	netdev, kuba

On 6/11/25 8:03 PM, Tony Nguyen wrote:
> To harmonize TX queue priority behavior between taprio and mqprio, and
> to fix these issues without breaking long-standing taprio use cases,
> this patch adds a new private flag, called reverse-tsn-txq-prio, to
> reverse the TX queue priority. It makes queue 3 the highest and queue 0
> the lowest, reusing the TX arbitration logic already used by mqprio.
Isn't the above quite the opposite of what Vladimir asked in
https://lore.kernel.org/all/20250214113815.37ttoor3isrt34dg@skbuf/ ?

"""
I would expect that for uniform behavior, you would force the users a
little bit to adopt the new TX scheduling mode in taprio, otherwise any
configuration with preemptible traffic classes would be rejected by the
driver.
"""

I don't see him commenting on later version, @Vladimir: does this fits you?

Thanks,

Paolo



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

* Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-17 10:06   ` Paolo Abeni
@ 2025-06-17 12:17     ` Vladimir Oltean
  2025-06-17 12:45       ` Paolo Abeni
  2025-06-18  6:35       ` Abdul Rahim, Faizal
  0 siblings, 2 replies; 15+ messages in thread
From: Vladimir Oltean @ 2025-06-17 12:17 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Tony Nguyen, Faizal Rahim, faizal.abdul.rahim, chwee.lin.choong,
	horms, vitaly.lifshits, dima.ruinskiy, Mor Bar-Gabay, davem,
	edumazet, andrew+netdev, netdev, kuba

Hi Paolo,

On Tue, Jun 17, 2025 at 12:06:14PM +0200, Paolo Abeni wrote:
> On 6/11/25 8:03 PM, Tony Nguyen wrote:
> > To harmonize TX queue priority behavior between taprio and mqprio, and
> > to fix these issues without breaking long-standing taprio use cases,
> > this patch adds a new private flag, called reverse-tsn-txq-prio, to
> > reverse the TX queue priority. It makes queue 3 the highest and queue 0
> > the lowest, reusing the TX arbitration logic already used by mqprio.
> Isn't the above quite the opposite of what Vladimir asked in
> https://lore.kernel.org/all/20250214113815.37ttoor3isrt34dg@skbuf/ ?
> 
> """
> I would expect that for uniform behavior, you would force the users a
> little bit to adopt the new TX scheduling mode in taprio, otherwise any
> configuration with preemptible traffic classes would be rejected by the
> driver.
> """
> 
> I don't see him commenting on later version, @Vladimir: does this fits you?

Indeed, sorry for disappearing from the patch review process.

I don't see the discrepancy between what Faizal implemented and what we
discussed. Specifically on the bit you quoted - patch "igc: add
preemptible queue support in taprio" refuses taprio schedules with
preemptible TCs if the user hasn't explicitly opted into
IGC_FLAG_TSN_REVERSE_TXQ_PRIO. If that private flag isn't set,
everything works as currently documented, just the new features are
gated.

The name of the private flag is debatable IMHO, because it's taprio
specific and the name doesn't reflect that (mqprio uses the "reverse"
priority assignment to TX queues by default, and this flag doesn't
change that). Also, "reverse" compared to what? Both operating modes can
equally be named "reverse". Maybe "taprio-standard-txq-priority" would
have been clearer regarding what the flag really does. Anyway, I don't
want to stir up a huge debate around naming if functionality-wise it's
the same thing, they have to maintain it, I don't.

Is there something I'm missing? It feels like it.

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

* Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-17 12:17     ` Vladimir Oltean
@ 2025-06-17 12:45       ` Paolo Abeni
  2025-06-17 12:48         ` Vladimir Oltean
  2025-06-18  6:35       ` Abdul Rahim, Faizal
  1 sibling, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2025-06-17 12:45 UTC (permalink / raw)
  To: Vladimir Oltean, Tony Nguyen, Faizal Rahim
  Cc: faizal.abdul.rahim, chwee.lin.choong, horms, vitaly.lifshits,
	dima.ruinskiy, Mor Bar-Gabay, davem, edumazet, andrew+netdev,
	netdev, kuba

On 6/17/25 2:17 PM, Vladimir Oltean wrote:
> On Tue, Jun 17, 2025 at 12:06:14PM +0200, Paolo Abeni wrote:
>> On 6/11/25 8:03 PM, Tony Nguyen wrote:
>>> To harmonize TX queue priority behavior between taprio and mqprio, and
>>> to fix these issues without breaking long-standing taprio use cases,
>>> this patch adds a new private flag, called reverse-tsn-txq-prio, to
>>> reverse the TX queue priority. It makes queue 3 the highest and queue 0
>>> the lowest, reusing the TX arbitration logic already used by mqprio.
>> Isn't the above quite the opposite of what Vladimir asked in
>> https://lore.kernel.org/all/20250214113815.37ttoor3isrt34dg@skbuf/ ?
>>
>> """
>> I would expect that for uniform behavior, you would force the users a
>> little bit to adopt the new TX scheduling mode in taprio, otherwise any
>> configuration with preemptible traffic classes would be rejected by the
>> driver.
>> """
>>
>> I don't see him commenting on later version, @Vladimir: does this fits you?
> 
> Indeed, sorry for disappearing from the patch review process.
> 
> I don't see the discrepancy between what Faizal implemented and what we
> discussed. Specifically on the bit you quoted - patch "igc: add
> preemptible queue support in taprio" refuses taprio schedules with
> preemptible TCs if the user hasn't explicitly opted into
> IGC_FLAG_TSN_REVERSE_TXQ_PRIO. If that private flag isn't set,
> everything works as currently documented, just the new features are
> gated.
> 
> The name of the private flag is debatable IMHO, because it's taprio
> specific and the name doesn't reflect that (mqprio uses the "reverse"
> priority assignment to TX queues by default, and this flag doesn't
> change that). Also, "reverse" compared to what? Both operating modes can
> equally be named "reverse". Maybe "taprio-standard-txq-priority" would
> have been clearer regarding what the flag really does. Anyway, I don't
> want to stir up a huge debate around naming if functionality-wise it's
> the same thing, they have to maintain it, I don't.
> 
> Is there something I'm missing? It feels like it.

I misread your original comment as a request to make the 'standard'
priority the default, and the current one reachable only enabling the
priv flag.

I see you are fine with the current status, so the answer to your
question is 'no' ;)

Paolo


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

* Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-17 12:45       ` Paolo Abeni
@ 2025-06-17 12:48         ` Vladimir Oltean
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Oltean @ 2025-06-17 12:48 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Tony Nguyen, Faizal Rahim, faizal.abdul.rahim, chwee.lin.choong,
	horms, vitaly.lifshits, dima.ruinskiy, Mor Bar-Gabay, davem,
	edumazet, andrew+netdev, netdev, kuba

On Tue, Jun 17, 2025 at 02:45:50PM +0200, Paolo Abeni wrote:
> > Is there something I'm missing? It feels like it.
> 
> I misread your original comment as a request to make the 'standard'
> priority the default, and the current one reachable only enabling the
> priv flag.
> 
> I see you are fine with the current status, so the answer to your
> question is 'no' ;)
> 
> Paolo

No, that would be a breaking change to their current documentation which
is all over the web. I just don't want that operating mode to continue
as such in combination with frame preemption.

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

* Re: [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support
  2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
                   ` (7 preceding siblings ...)
  2025-06-13  1:42 ` [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Jakub Kicinski
@ 2025-06-17 13:10 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 15+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-17 13:10 UTC (permalink / raw)
  To: Tony Nguyen
  Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
	faizal.abdul.rahim, faizal.abdul.rahim, chwee.lin.choong,
	vladimir.oltean, horms, vitaly.lifshits, dima.ruinskiy

Hello:

This series was applied to netdev/net-next.git (main)
by Tony Nguyen <anthony.l.nguyen@intel.com>:

On Wed, 11 Jun 2025 11:03:02 -0700 you wrote:
> Faizal Rahim says:
> 
> MAC Merge support for frame preemption was previously added for igc:
> https://lore.kernel.org/netdev/20250418163822.3519810-1-anthony.l.nguyen@intel.com/
> 
> This series builds on that work and adds support for:
> - Harmonizing taprio and mqprio queue priority behavior, based on past
>   discussions and suggestions:
>   https://lore.kernel.org/all/20250214102206.25dqgut5tbak2rkz@skbuf/
> - Enabling preemptible queue support for both taprio and mqprio, with
>   priority harmonization as a prerequisite.
> 
> [...]

Here is the summary with links:
  - [net-next,1/7] igc: move TXDCTL and RXDCTL related macros
    https://git.kernel.org/netdev/net-next/c/fe4d9e8394ff
  - [net-next,2/7] igc: add DCTL prefix to related macros
    https://git.kernel.org/netdev/net-next/c/4cdb4ef8a9ff
  - [net-next,3/7] igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK
    https://git.kernel.org/netdev/net-next/c/e35ba6d3c6c3
  - [net-next,4/7] igc: assign highest TX queue number as highest priority in mqprio
    https://git.kernel.org/netdev/net-next/c/650a2fe79538
  - [net-next,5/7] igc: add private flag to reverse TX queue priority in TSN mode
    https://git.kernel.org/netdev/net-next/c/e395f6a690d8
  - [net-next,6/7] igc: add preemptible queue support in taprio
    https://git.kernel.org/netdev/net-next/c/17643482e9ff
  - [net-next,7/7] igc: add preemptible queue support in mqprio
    https://git.kernel.org/netdev/net-next/c/a7d45bcfde3c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode
  2025-06-17 12:17     ` Vladimir Oltean
  2025-06-17 12:45       ` Paolo Abeni
@ 2025-06-18  6:35       ` Abdul Rahim, Faizal
  1 sibling, 0 replies; 15+ messages in thread
From: Abdul Rahim, Faizal @ 2025-06-18  6:35 UTC (permalink / raw)
  To: Vladimir Oltean, Paolo Abeni
  Cc: Tony Nguyen, faizal.abdul.rahim, chwee.lin.choong, horms,
	vitaly.lifshits, dima.ruinskiy, Mor Bar-Gabay, davem, edumazet,
	andrew+netdev, netdev, kuba


Hi Vladimir,

Thanks for your feedback.

On 17/6/2025 8:17 pm, Vladimir Oltean wrote:
> Hi Paolo,
> 
> On Tue, Jun 17, 2025 at 12:06:14PM +0200, Paolo Abeni wrote:
>> On 6/11/25 8:03 PM, Tony Nguyen wrote:
>>> To harmonize TX queue priority behavior between taprio and mqprio, and
>>> to fix these issues without breaking long-standing taprio use cases,
>>> this patch adds a new private flag, called reverse-tsn-txq-prio, to
>>> reverse the TX queue priority. It makes queue 3 the highest and queue 0
>>> the lowest, reusing the TX arbitration logic already used by mqprio.
>> Isn't the above quite the opposite of what Vladimir asked in
>> https://lore.kernel.org/all/20250214113815.37ttoor3isrt34dg@skbuf/ ?
>>
>> """
>> I would expect that for uniform behavior, you would force the users a
>> little bit to adopt the new TX scheduling mode in taprio, otherwise any
>> configuration with preemptible traffic classes would be rejected by the
>> driver.
>> """
>>
>> I don't see him commenting on later version, @Vladimir: does this fits you?
> 
> Indeed, sorry for disappearing from the patch review process.
> 
> I don't see the discrepancy between what Faizal implemented and what we
> discussed. Specifically on the bit you quoted - patch "igc: add
> preemptible queue support in taprio" refuses taprio schedules with
> preemptible TCs if the user hasn't explicitly opted into
> IGC_FLAG_TSN_REVERSE_TXQ_PRIO. If that private flag isn't set,
> everything works as currently documented, just the new features are
> gated.
> 
> The name of the private flag is debatable IMHO, because it's taprio
> specific and the name doesn't reflect that (mqprio uses the "reverse"
> priority assignment to TX queues by default, and this flag doesn't
> change that). Also, "reverse" compared to what? Both operating modes can
Compared to the default Tx queue priority in TSN mode, where Tx q0 has the 
highest priority and q3 the lowest. My thinking behind the naming was based 
on how the relevant register fields are configured.

Snippet of i226 documentation:
"While in TSN mode each transmit queue is assigned a priority level by the 
TxQ_Priority fields in the TxARB register"
"TxQ_Priority_0: The transmit queue that is assigned as priority 0 (highest 
priority). Default is queue 0"
"TxQ_Priority_3: The transmit queue that is assigned as priority 3 (lowest 
priority). Default is queue 3."

> equally be named "reverse". 
True —  mqprio already uses the reverse mapping by default even without 
this private flag, which could feel inconsistent.

> Maybe "taprio-standard-txq-priority" would
> have been clearer regarding what the flag really does.
I’m okay with that suggestion, though I’m not sure it makes things clearer. 
What’s considered “standard” may depend on context — for IGC users who 
haven’t worked with other NICs, the existing default priority mapping might 
feel like the standard. I’m definitely in favor of improving readability 
and maintenance, but I’m still unsure what name best reflects that balance.


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

end of thread, other threads:[~2025-06-18  6:35 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 18:03 [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 1/7] igc: move TXDCTL and RXDCTL related macros Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 2/7] igc: add DCTL prefix to " Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 3/7] igc: refactor TXDCTL macros to use FIELD_PREP and GEN_MASK Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 4/7] igc: assign highest TX queue number as highest priority in mqprio Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 5/7] igc: add private flag to reverse TX queue priority in TSN mode Tony Nguyen
2025-06-17 10:06   ` Paolo Abeni
2025-06-17 12:17     ` Vladimir Oltean
2025-06-17 12:45       ` Paolo Abeni
2025-06-17 12:48         ` Vladimir Oltean
2025-06-18  6:35       ` Abdul Rahim, Faizal
2025-06-11 18:03 ` [PATCH net-next 6/7] igc: add preemptible queue support in taprio Tony Nguyen
2025-06-11 18:03 ` [PATCH net-next 7/7] igc: add preemptible queue support in mqprio Tony Nguyen
2025-06-13  1:42 ` [PATCH net-next 0/7][pull request] igc: harmonize queue priority and add preemptible queue support Jakub Kicinski
2025-06-17 13:10 ` patchwork-bot+netdevbpf

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