[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6136a4bd-1818-f875-5355-547ffb3bd25b@linux.intel.com>
Date: Tue, 4 Jul 2023 12:34:58 +0300
From: "naamax.meir" <naamax.meir@...ux.intel.com>
To: Florian Kauer <florian.kauer@...utronix.de>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Tan Tee Min <tee.min.tan@...ux.intel.com>,
Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>,
Aravindhan Gunasekaran <aravindhan.gunasekaran@...el.com>,
Malli C <mallikarjuna.chilakala@...el.com>
Cc: netdev@...r.kernel.org, kurt@...utronix.de,
intel-wired-lan@...ts.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH net v2 4/6] igc: No strict mode in pure
launchtime/CBS offload
On 6/19/2023 13:08, Florian Kauer wrote:
> The flags IGC_TXQCTL_STRICT_CYCLE and IGC_TXQCTL_STRICT_END
> prevent the packet transmission over slot and cycle boundaries.
> This is important for taprio offload where the slots and
> cycles correspond to the slots and cycles configured for the
> network.
>
> However, the Qbv offload feature of the i225 is also used for
> enabling TX launchtime / ETF offload. In that case, however,
> the cycle has no meaning for the network and is only used
> internally to adapt the base time register after a second has
> passed.
>
> Enabling strict mode in this case would unneccesarily prevent
> the transmission of certain packets (i.e. at the boundary of a
> second) and thus interfers with the ETF qdisc that promises
> transmission at a certain point in time.
>
> Similar to ETF, this also applies to CBS offload that also should
> not be influenced by strict mode unless taprio offload would be
> enabled at the same time.
>
> This fully reverts
> commit d8f45be01dd9 ("igc: Use strict cycles for Qbv scheduling")
> but its commit message only describes what was already implemented
> before that commit. The difference to a plain revert of that commit
> is that it now copes with the base_time = 0 case that was fixed with
> commit e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv")
>
> In particular, enabling strict mode leads to TX hang situations
> under high traffic if taprio is applied WITHOUT taprio offload
> but WITH ETF offload, e.g. as in
>
> sudo tc qdisc replace dev enp1s0 parent root handle 100 taprio \
> num_tc 1 \
> map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
> queues 1@0 \
> base-time 0 \
> sched-entry S 01 300000 \
> flags 0x1 \
> txtime-delay 500000 \
> clockid CLOCK_TAI
> sudo tc qdisc replace dev enp1s0 parent 100:1 etf \
> clockid CLOCK_TAI \
> delta 500000 \
> offload \
> skip_sock_check
>
> and traffic generator
>
> sudo trafgen -i traffic.cfg -o enp1s0 --cpp -n0 -q -t1400ns
>
> with traffic.cfg
>
> #define ETH_P_IP 0x0800
>
> {
> /* Ethernet Header */
> 0x30, 0x1f, 0x9a, 0xd0, 0xf0, 0x0e, # MAC Dest - adapt as needed
> 0x24, 0x5e, 0xbe, 0x57, 0x2e, 0x36, # MAC Src - adapt as needed
> const16(ETH_P_IP),
>
> /* IPv4 Header */
> 0b01000101, 0, # IPv4 version, IHL, TOS
> const16(1028), # IPv4 total length (UDP length + 20 bytes (IP header))
> const16(2), # IPv4 ident
> 0b01000000, 0, # IPv4 flags, fragmentation off
> 64, # IPv4 TTL
> 17, # Protocol UDP
> csumip(14, 33), # IPv4 checksum
>
> /* UDP Header */
> 10, 0, 48, 1, # IP Src - adapt as needed
> 10, 0, 48, 10, # IP Dest - adapt as needed
> const16(5555), # UDP Src Port
> const16(6666), # UDP Dest Port
> const16(1008), # UDP length (UDP header 8 bytes + payload length)
> csumudp(14, 34), # UDP checksum
>
> /* Payload */
> fill('W', 1000),
> }
>
> and the observed message with that is for example
>
> igc 0000:01:00.0 enp1s0: Detected Tx Unit Hang
> Tx Queue <0>
> TDH <d0>
> TDT <f0>
> next_to_use <f0>
> next_to_clean <d0>
> buffer_info[next_to_clean]
> time_stamp <ffff661f>
> next_to_watch <00000000245a4efb>
> jiffies <ffff6e48>
> desc.status <1048000>
>
> Fixes: d8f45be01dd9 ("igc: Use strict cycles for Qbv scheduling")
> Signed-off-by: Florian Kauer <florian.kauer@...utronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@...utronix.de>
> ---
> drivers/net/ethernet/intel/igc/igc_tsn.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
Powered by blists - more mailing lists