netdev - Re: [PATCH iwl-net v2 3/3] igc: Fix qbv tx latency by setting gtxoffset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a5iou7ty.fsf@intel.com>
Date: Wed, 10 Jul 2024 16:48:41 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Jesse Brandeburg
 <jesse.brandeburg@...el.com>, Tony
 Nguyen <anthony.l.nguyen@...el.com>, Simon Horman <horms@...nel.org>,
 Richard Cochran <richardcochran@...il.com>, Paul Menzel
 <pmenzel@...gen.mpg.de>, Sasha Neftin <sasha.neftin@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org, Faizal Rahim
 <faizal.abdul.rahim@...ux.intel.com>
Subject: Re: [PATCH iwl-net v2 3/3] igc: Fix qbv tx latency by setting
 gtxoffset

Faizal Rahim <faizal.abdul.rahim@...ux.intel.com> writes:

> A large tx latency issue was discovered during testing when only QBV was
> enabled. The issue occurs because gtxoffset was not set when QBV is
> active, it was only set when launch time is active.
>
> The patch "igc: Correct the launchtime offset" only sets gtxoffset when
> the launchtime_enable field is set by the user. Enabling launchtime_enable
> ultimately sets the register IGC_TXQCTL_QUEUE_MODE_LAUNCHT (referred to as
> LaunchT in the SW user manual).
>
> Section 7.5.2.6 of the IGC i225/6 SW User Manual Rev 1.2.4 states:
> "The latency between transmission scheduling (launch time) and the
> time the packet is transmitted to the network is listed in Table 7-61."
>
> However, the patch misinterprets the phrase "launch time" in that section
> by assuming it specifically refers to the LaunchT register, whereas it
> actually denotes the generic term for when a packet is released from the
> internal buffer to the MAC transmit logic.
>
> This launch time, as per that section, also implicitly refers to the QBV
> gate open time, where a packet waits in the buffer for the QBV gate to
> open. Therefore, latency applies whenever QBV is in use. TSN features such
> as QBU and QAV reuse QBV, making the latency universal to TSN features.
>
> Discussed with i226 HW owner (Shalev, Avi) and we were in agreement that
> the term "launch time" used in Section 7.5.2.6 is not clear and can be
> easily misinterpreted. Avi will update this section to:
> "When TQAVCTRL.TRANSMIT_MODE = TSN, the latency between transmission
> scheduling and the time the packet is transmitted to the network is listed
> in Table 7-61."
>
> Fix this issue by using igc_tsn_is_tx_mode_in_tsn() as a condition to
> write to gtxoffset, aligning with the newly updated SW User Manual.
>
> Tested:
> 1. Enrol taprio on talker board
>    base-time 0
>    cycle-time 1000000
>    flags 0x2
>    index 0 cmd S gatemask 0x1 interval1
>    index 0 cmd S gatemask 0x1 interval2
>
>    Note:
>    interval1 = interval for a 64 bytes packet to go through
>    interval2 = cycle-time - interval1
>
> 2. Take tcpdump on listener board
>
> 3. Use udp tai app on talker to send packets to listener
>
> 4. Check the timestamp on listener via wireshark
>
> Test Result:
> 100 Mbps: 113 ~193 ns
> 1000 Mbps: 52 ~ 84 ns
> 2500 Mbps: 95 ~ 223 ns
>
> Note that the test result is similar to the patch "igc: Correct the
> launchtime offset".
>
> Fixes: 790835fcc0cb ("igc: Correct the launchtime offset")
> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
> Reviewed-by: Simon Horman <horms@...nel.org>
> ---

Good catch.

Acked-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>

>  drivers/net/ethernet/intel/igc/igc_tsn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 9fafe275f30f..efe13a9350ca 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -61,7 +61,7 @@ void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter)
>  	struct igc_hw *hw = &adapter->hw;
>  	u16 txoffset;
>  
> -	if (!is_any_launchtime(adapter))
> +	if (!igc_tsn_is_tx_mode_in_tsn(adapter))
>  		return;
>  
>  	switch (adapter->link_speed) {
> -- 
> 2.25.1
>


Cheers,
-- 
Vinicius