net: tcp: Change SYN FIN to use send_data_timer

[shrek-wang]

The send_queue was used as SYN/FIN packet retransmission. Before the SYN/FIN being ACKed and dequeue-ed, the following packets in the send_queue cannot be sent out. That's why Zephyr had to send a FIN+ACK instead of a duplicated ACK-only in FINWAIT1, CLOSING. In fact, we can take SYN/FIN as kind of data and use the same send_data_timer for retransmission, like other OSes do. This way, the send_queue is simply used for local traffics.
Benefits (in theory):

1. The code is easier,
2. TxPkt performance is better after skipping enq/deq send_queue,
3. The struct tcp{} node is a few bytes smaller, saving memory.

[shrek-wang]

@hakehuang , could you please help checking it on the physical boards? Thanks!

[rlubos]

Thanks, having (in practice) two retransmission timers was indeed pretty confusing, the change looks quite reasonalable, another step in the right direction 👍

[rlubos]

Regarding twister failure - the removed flag is still referenced in the net shell:

zephyr/subsys/net/lib/shell/conn.c

Line 151 in 5601af3

conn->in_retransmission, conn->in_connect, conn->in_close,

[hakehuang]

@hakehuang , could you please help checking it on the physical boards? Thanks!

board testing is OK on mimxrt1060_evk@C, and with max-pro testing, no regression issues found.

[shrek-wang]

Regarding twister failure - the removed flag is still referenced in the net shell:

zephyr/subsys/net/lib/shell/conn.c

Line 151 in 5601af3

conn->in_retransmission, conn->in_connect, conn->in_close,

Thanks @rlubos , I didn't notice that during my local run. Sorry.
After the change, the 'send_queue' won't be used for all the TCP packets transmission while only for local traffics. If you agree, I'd like to keep the tcp_send_list_cb() there for now and only fix the issue like this:
replace the 'conn->in_retransmission' with 'conn->data_mode == TCP_DATA_MODE_RESEND ? 1 : 0'.
In fact, the SYN/FIN 'in_retransmission' or not can be deduced via checking the TCP State.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rlubos]

Regarding twister failure - the removed flag is still referenced in the net shell:

zephyr/subsys/net/lib/shell/conn.c

Line 151 in 5601af3

conn->in_retransmission, conn->in_connect, conn->in_close,

Thanks @rlubos , I didn't notice that during my local run. Sorry. After the change, the 'send_queue' won't be used for all the TCP packets transmission while only for local traffics. If you agree, I'd like to keep the tcp_send_list_cb() there for now and only fix the issue like this: replace the 'conn->in_retransmission' with 'conn->data_mode == TCP_DATA_MODE_RESEND ? 1 : 0'. In fact, the SYN/FIN 'in_retransmission' or not can be deduced via checking the TCP State.

Sounds ok

[rlubos]

Looks good, thanks