By the way, I'd like to know your opinion: * should I add a sample implementation of http/1.1 compatible web-server to this patch series?
This can be used as a base for other implementations like firmware upgrade web-server used by some vendors. Mikhail Kshevetskiy On 17.08.2024 18:58, Simon Glass wrote: > Hi Mikhail, > > On Wed, 14 Aug 2024 at 04:32, Mikhail Kshevetskiy > <mikhail.kshevets...@iopsys.eu> wrote: >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevets...@iopsys.eu> >> --- >> net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- >> 1 file changed, 37 insertions(+), 33 deletions(-) >> > Reviewed-by: Simon Glass <s...@chromium.org> > > nits below > >> diff --git a/net/tcp.c b/net/tcp.c >> index 2c34556c26d..7014d5b4f43 100644 >> --- a/net/tcp.c >> +++ b/net/tcp.c >> @@ -527,10 +527,37 @@ void net_set_syn_options(struct tcp_stream *tcp, union >> tcp_build_pkt *b) >> b->ip.end = TCP_O_END; >> } >> >> +const char *tcpflags_to_str(char tcpflags, char *buf, int size) >> +{ >> + int i = 0, len; >> + char *orig = buf; >> + const struct { >> + int bit; >> + const char *name; >> + } desc[] = {{TCP_RST, "RST"}, {TCP_SYN, "SYN"}, {TCP_PUSH, "PSH"}, >> + {TCP_FIN, "FIN"}, {TCP_ACK, "ACK"}, {0, NULL}}; >> + >> + *orig = '\0'; >> + while (desc[i].name != NULL) { > try to avoid comparing with NULL or 0 > >> + len = strlen(desc[i].name); >> + if (size <= len + 1) >> + break; >> + if (buf != orig) { >> + *buf++ = ','; >> + size--; >> + } >> + strcpy(buf, desc[i].name); >> + buf += len; >> + size -= len; >> + } >> + return orig; >> +} >> + >> int tcp_set_tcp_header(struct tcp_stream *tcp, uchar *pkt, int payload_len, >> u8 action, u32 tcp_seq_num, u32 tcp_ack_num) >> { >> union tcp_build_pkt *b = (union tcp_build_pkt *)pkt; >> + char buf[24]; >> int pkt_hdr_len; >> int pkt_len; >> int tcp_len; >> @@ -540,55 +567,32 @@ int tcp_set_tcp_header(struct tcp_stream *tcp, uchar >> *pkt, int payload_len, >> * 4 bits reserved options >> */ >> b->ip.hdr.tcp_flags = action; >> - pkt_hdr_len = IP_TCP_HDR_SIZE; >> b->ip.hdr.tcp_hlen = >> SHIFT_TO_TCPHDRLEN_FIELD(LEN_B_TO_DW(TCP_HDR_SIZE)); >> >> switch (action) { >> case TCP_SYN: >> debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:SYN (%pI4, %pI4, sq=%u, ak=%u)\n", >> - &tcp->rhost, &net_ip, >> - tcp_seq_num, tcp_ack_num); >> + "TCP Hdr:%s (%pI4, %pI4, s=%u, a=%u)\n", >> + tcpflags_to_str(action, buf, sizeof(buf)), >> + &tcp->rhost, &net_ip, tcp_seq_num, tcp_ack_num); >> net_set_syn_options(tcp, b); >> pkt_hdr_len = IP_TCP_O_SIZE; >> break; >> - case TCP_SYN | TCP_ACK: >> - case TCP_ACK: >> - pkt_hdr_len = IP_HDR_SIZE + net_set_ack_options(tcp, b); >> - b->ip.hdr.tcp_flags = action; >> - debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:ACK (%pI4, %pI4, s=%u, a=%u, A=%x)\n", >> - &tcp->rhost, &net_ip, tcp_seq_num, tcp_ack_num, >> - action); >> - break; >> - case TCP_FIN: >> - debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:FIN (%pI4, %pI4, s=%u, a=%u)\n", >> - &tcp->rhost, &net_ip, tcp_seq_num, tcp_ack_num); >> - payload_len = 0; >> - pkt_hdr_len = IP_TCP_HDR_SIZE; >> - break; >> case TCP_RST | TCP_ACK: >> case TCP_RST: >> debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:RST (%pI4, %pI4, s=%u, a=%u)\n", >> + "TCP Hdr:%s (%pI4, %pI4, s=%u, a=%u)\n", >> + tcpflags_to_str(action, buf, sizeof(buf)), >> &tcp->rhost, &net_ip, tcp_seq_num, tcp_ack_num); >> + pkt_hdr_len = IP_TCP_HDR_SIZE; >> break; >> - /* Notify connection closing */ >> - case (TCP_FIN | TCP_ACK): >> - case (TCP_FIN | TCP_ACK | TCP_PUSH): >> - debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:FIN ACK PSH(%pI4, %pI4, s=%u, a=%u, >> A=%x)\n", >> - &tcp->rhost, &net_ip, >> - tcp_seq_num, tcp_ack_num, action); >> - fallthrough; >> default: >> pkt_hdr_len = IP_HDR_SIZE + net_set_ack_options(tcp, b); >> - b->ip.hdr.tcp_flags = action | TCP_PUSH | TCP_ACK; >> debug_cond(DEBUG_DEV_PKT, >> - "TCP Hdr:dft (%pI4, %pI4, s=%u, a=%u, A=%x)\n", >> - &tcp->rhost, &net_ip, >> - tcp_seq_num, tcp_ack_num, action); >> + "TCP Hdr:%s (%pI4, %pI4, s=%u, a=%u)\n", >> + tcpflags_to_str(action, buf, sizeof(buf)), >> + &tcp->rhost, &net_ip, tcp_seq_num, tcp_ack_num); >> + break; >> } >> >> pkt_len = pkt_hdr_len + payload_len; >> -- >> 2.39.2 >> > Regards, > Simon