net: tcp: Remove NULL-pkt support for tcp_in() #89862
Conversation
github-actions
bot
requested review from
jukkar,
pdgendt,
rlubos,
ssharks and
tbursztyka
| ret = tcp_out_ext(conn, FIN | ACK, NULL, | ||
| conn->seq + conn->unacked_len); | ||
| if (ret == 0) { | ||
| conn_seq(conn, + 1); |
There was a problem hiding this comment.
Are those kind of changes necessary? It just blurs the diff. At the very least they should be in a separate commit.
There was a problem hiding this comment.
Sorry, this change is not related with this PR. That was because I saw some check failures when I run the check_compliance.py to my patch, and then I modified all similar issues throughout the tcp.c.
I will remove this line of change. Thanks, @rlubos.
subsys/net/ip/tcp.c
Outdated
| * FIN & ACK -> TCP_TIME_WAIT | ||
| */ | ||
| if (th) { | ||
| do { |
There was a problem hiding this comment.
Why wrapping this code in a dummy do { } while (0) loop? I think we should be consistent and just reduce the indentation everywhere, let's do it once and for good.
subsys/net/ip/tcp.c
Outdated
| return 0; | ||
| } | ||
|
|
||
| static int tcp_start_handshaking(struct tcp *conn) |
There was a problem hiding this comment.
nit, but I'd call it tcp_start_handshake(), present form.
|
Thanks, @rlubos. I will update the code accordingly. |
|
@jukkar, the twister check failures look confusing - not much like related with this PR changes? Could you please help taking a look? Thanks! |
subsys/net/ip/tcp.c
Outdated
| tcp_out(conn, SYN | ACK); | ||
| conn->send_options.mss_found = false; | ||
| conn_seq(conn, + 1); | ||
| conn_seq(conn, 1); |
There was a problem hiding this comment.
That's similar to #89862 (comment), same in other places below.
subsys/net/ip/tcp.c
Outdated
| } | ||
| } | ||
| break; | ||
| } break; |
There was a problem hiding this comment.
break; should be in a separate line according to CS I think.
subsys/net/ip/tcp.c
Outdated
| case TCP_TIME_WAIT: | ||
| if (th) { | ||
| int32_t new_len = tcp_compute_new_length(conn, th, len, true); | ||
| } break; |
subsys/net/ip/tcp.c
Outdated
| } | ||
| } | ||
| break; | ||
| } break; |
|
@rlubos , it's a bit weird that I always got compliance-check failure if keeping the '+ 1'. I tried multiple times and finally succeeded after removing the '+'. |
The NULL-pkt parameter for tcp_in() was designed for generating a SYN packet to start the 1st TCP handshake. It is only used in net_tcp_connect() and tp_input(). To simplify the tcp_in() code logic and make it better under- standable, a tcp_start_handshake() is added for net_tcp_connect() and tp_input() to use. Thus, the tcp_in() only handles the in- coming TCP packets. Signed-off-by: Shrek Wang <[email protected]>
|
The NULL-pkt parameter for tcp_in() was designed for generating a SYN packet to start the 1st TCP handshake. It is only used in net_tcp_connect() and tp_input().
To simplify the tcp_in() code logic and make it better under- standable, a tcp_start_handshake() is added for net_tcp_connect() and tp_input() to use. Thus, the tcp_in() only handles the in- coming TCP packets.