lkml.org 
[lkml]   [2023]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Intel-wired-lan] [PATCH net v2 6/6] igc: Fix inserting of empty frame for launchtime
From
On 6/19/2023 13:08, Florian Kauer wrote:
> The insertion of an empty frame was introduced with
> commit db0b124f02ba ("igc: Enhance Qbv scheduling by using first flag bit")
> in order to ensure that the current cycle has at least one packet if
> there is some packet to be scheduled for the next cycle.
>
> However, the current implementation does not properly check if
> a packet is already scheduled for the current cycle. Currently,
> an empty packet is always inserted if and only if
> txtime >= end_of_cycle && txtime > last_tx_cycle
> but since last_tx_cycle is always either the end of the current
> cycle (end_of_cycle) or the end of a previous cycle, the
> second part (txtime > last_tx_cycle) is always true unless
> txtime == last_tx_cycle.
>
> What actually needs to be checked here is if the last_tx_cycle
> was already written within the current cycle, so an empty frame
> should only be inserted if and only if
> txtime >= end_of_cycle && end_of_cycle > last_tx_cycle.
>
> This patch does not only avoid an unnecessary insertion, but it
> can actually be harmful to insert an empty packet if packets
> are already scheduled in the current cycle, because it can lead
> to a situation where the empty packet is actually processed
> as the first packet in the upcoming cycle shifting the packet
> with the first_flag even one cycle into the future, finally leading
> to a TX hang.
>
> The TX hang can be reproduced on a i225 with:
>
> 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 <32>
> TDT <3c>
> next_to_use <3c>
> next_to_clean <32>
> buffer_info[next_to_clean]
> time_stamp <ffff26a8>
> next_to_watch <00000000632a1828>
> jiffies <ffff27f8>
> desc.status <1048000>
>
> Fixes: db0b124f02ba ("igc: Enhance Qbv scheduling by using first flag bit")
> Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Naama Meir <naamax.meir@linux.intel.com>

\
 
 \ /
  Last update: 2023-07-06 13:37    [W:0.165 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site