Re: [Intel-wired-lan] [PATCH iwl-next v2] igc: Change Tx mode for MQPRIO offloading

[Kurt Kanzenbach <kurt@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 5536 bytes --]

On Mon Feb 24 2025, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>> Kurt Kanzenbach
>> Sent: Monday, February 24, 2025 11:05 AM
>> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
>> <przemyslaw.kitszel@intel.com>; Faizal Rahim
>> <faizal.abdul.rahim@linux.intel.com>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
>> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
>> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Sebastian
>> Andrzej Siewior <bigeasy@linutronix.de>; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; Kurt Kanzenbach <kurt@linutronix.de>
>> Subject: [Intel-wired-lan] [PATCH iwl-next v2] igc: Change Tx mode for
>> MQPRIO offloading
>> 
>> The current MQPRIO offload implementation uses the legacy TSN Tx mode. In
>> this mode the hardware uses four packet buffers and considers queue
>> priorities.
>> 
>> In order to harmonize the TAPRIO implementation with MQPRIO, switch to
>> the regular TSN Tx mode. In addition to the legacy mode, transmission is
>> always coupled to Qbv. The driver already has mechanisms to use a dummy
>> schedule of 1 second with all gates open for ETF. Simply use this for MQPRIO
>> too.
>> 
>> This reduces code and makes it easier to add support for frame preemption
>> later.
>> 
>> While at it limit the netdev_tc calls to MQPRIO only.
>> 
>> Tested on i225 with real time application using high priority queue, iperf3
>> using low priority queue and network TAP device.
>> 
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>> ---
>> Changes in v2:
>> - Add comma to commit message (Faizal)
>> - Simplify if condition (Faizal)
>> - Link to v1: https://lore.kernel.org/r/20250217-igc_mqprio_tx_mode-v1-1-
>> 3a402fe1f326@linutronix.de
>> ---
>>  drivers/net/ethernet/intel/igc/igc.h      |  4 +---
>>  drivers/net/ethernet/intel/igc/igc_main.c | 18 +++++++++++++-
>> drivers/net/ethernet/intel/igc/igc_tsn.c  | 40 ++-----------------------------
>>  3 files changed, 20 insertions(+), 42 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/igc/igc.h
>> b/drivers/net/ethernet/intel/igc/igc.h
>> index
>> cd1d7b6c1782352094f6867a31b6958c929bbbf4..16d85bdf55a7e9c412c4
>> 7acf727bca6bc7154c61 100644
>> --- a/drivers/net/ethernet/intel/igc/igc.h
>> +++ b/drivers/net/ethernet/intel/igc/igc.h
>> @@ -388,11 +388,9 @@ extern char igc_driver_name[];
>>  #define IGC_FLAG_RX_LEGACY		BIT(16)
>>  #define IGC_FLAG_TSN_QBV_ENABLED	BIT(17)
>>  #define IGC_FLAG_TSN_QAV_ENABLED	BIT(18)
>> -#define IGC_FLAG_TSN_LEGACY_ENABLED	BIT(19)
>> 
>>  #define IGC_FLAG_TSN_ANY_ENABLED				\
>> -	(IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED |
>> 	\
>> -	 IGC_FLAG_TSN_LEGACY_ENABLED)
>> +	(IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED)
>> 
>>  #define IGC_FLAG_RSS_FIELD_IPV4_UDP	BIT(6)
>>  #define IGC_FLAG_RSS_FIELD_IPV6_UDP	BIT(7)
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>> b/drivers/net/ethernet/intel/igc/igc_main.c
>> index
>> 3044392e8ded8619434040b9ccaa6b1babdbf685..0f44b0a6c166ae8aa798
>> 93ea87f706be5d94397c 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -6678,13 +6678,14 @@ static int igc_tsn_enable_mqprio(struct
>> igc_adapter *adapter,
>>  				 struct tc_mqprio_qopt_offload *mqprio)  {
>>  	struct igc_hw *hw = &adapter->hw;
>> -	int i;
>> +	int err, i;
>> 
>>  	if (hw->mac.type != igc_i225)
>>  		return -EOPNOTSUPP;
>> 
>>  	if (!mqprio->qopt.num_tc) {
>>  		adapter->strict_priority_enable = false;
>> +		netdev_reset_tc(adapter->netdev);
>>  		goto apply;
>>  	}
>> 
>> @@ -6715,6 +6716,21 @@ static int igc_tsn_enable_mqprio(struct
>> igc_adapter *adapter,
>>  	igc_save_mqprio_params(adapter, mqprio->qopt.num_tc,
>>  			       mqprio->qopt.offset);
>> 
>> +	err = netdev_set_num_tc(adapter->netdev, adapter->num_tc);
>> +	if (err)
>> +		return err;
>> +
>> +	for (i = 0; i < adapter->num_tc; i++) {
>> +		err = netdev_set_tc_queue(adapter->netdev, i, 1,
>> +					  adapter->queue_per_tc[i]);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>> +	/* In case the card is configured with less than four queues. */
>> +	for (; i < IGC_MAX_TX_QUEUES; i++)
>> +		adapter->queue_per_tc[i] = i;
>> +
>>  	mqprio->qopt.hw = TC_MQPRIO_HW_OFFLOAD_TCS;
>> 
>>  apply:
>> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
>> b/drivers/net/ethernet/intel/igc/igc_tsn.c
>> index
>> 1e44374ca1ffbb86e9893266c590f318984ef574..7c28f3e7bb576f0e6a21c8
>> 83e934ede4d53096f4 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
>> @@ -37,18 +37,13 @@ static unsigned int igc_tsn_new_flags(struct
>> igc_adapter *adapter)  {
>>  	unsigned int new_flags = adapter->flags &
>> ~IGC_FLAG_TSN_ANY_ENABLED;
>> 
>> -	if (adapter->taprio_offload_enable)
>> -		new_flags |= IGC_FLAG_TSN_QBV_ENABLED;
>> -
>> -	if (is_any_launchtime(adapter))
>> +	if (adapter->taprio_offload_enable || is_any_launchtime(adapter) ||
>> +	    adapter->strict_priority_enable)
> Isn't  sequence of:
> if (adapter->taprio_offload_enable || adapter->strict_priority_enable || is_any_launchtime(adapter))
> faster statistically?

I don't think it's faster, because it is unlikely that
strict_priority_enable is true. Most people do use taprio or
etf. Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]