Message ID | 20240814103145.1347645-8-mikhail.kshevetskiy@iopsys.eu |
---|---|
State | Superseded |
Delegated to: | Ramon Fried |
Headers | show |
Series | net: tcp: improve tcp support | expand |
Hi Mikhail, On Wed, 14 Aug 2024 at 04:32, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote: > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > --- > net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- > 1 file changed, 37 insertions(+), 33 deletions(-) > Reviewed-by: Simon Glass <sjg@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
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.kshevetskiy@iopsys.eu> wrote: >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> >> --- >> net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- >> 1 file changed, 37 insertions(+), 33 deletions(-) >> > Reviewed-by: Simon Glass <sjg@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
Hi Mikhail, On Fri, 23 Aug 2024 at 09:12, Mikhail Kshevetskiy <mikhail.kshevetskiy@genexis.eu> wrote: > > 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. Do you mean using the sandbox emulator? Yes I think that would be very helpful. Regards, Simon > > Mikhail Kshevetskiy > > > On 17.08.2024 18:58, Simon Glass wrote: > > Hi Mikhail, > > > > On Wed, 14 Aug 2024 at 04:32, Mikhail Kshevetskiy > > <mikhail.kshevetskiy@iopsys.eu> wrote: > >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > >> --- > >> net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- > >> 1 file changed, 37 insertions(+), 33 deletions(-) > >> > > Reviewed-by: Simon Glass <sjg@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
actually I know nothing about sandbox emulator On 23.08.2024 23:47, Simon Glass wrote: > Hi Mikhail, > > On Fri, 23 Aug 2024 at 09:12, Mikhail Kshevetskiy > <mikhail.kshevetskiy@genexis.eu> wrote: >> 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. > Do you mean using the sandbox emulator? Yes I think that would be very helpful. > > Regards, > Simon > > >> Mikhail Kshevetskiy >> >> >> On 17.08.2024 18:58, Simon Glass wrote: >>> Hi Mikhail, >>> >>> On Wed, 14 Aug 2024 at 04:32, Mikhail Kshevetskiy >>> <mikhail.kshevetskiy@iopsys.eu> wrote: >>>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> >>>> --- >>>> net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- >>>> 1 file changed, 37 insertions(+), 33 deletions(-) >>>> >>> Reviewed-by: Simon Glass <sjg@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
Hi Mikhail, On Sat, 24 Aug 2024 at 06:30, Mikhail Kshevetskiy <mikhail.kshevetskiy@genexis.eu> wrote: > > actually I know nothing about sandbox emulator This is the foundation of U-Boot's testing [1]. See for example test/cmd/wget.c > > > On 23.08.2024 23:47, Simon Glass wrote: > > Hi Mikhail, > > > > On Fri, 23 Aug 2024 at 09:12, Mikhail Kshevetskiy > > <mikhail.kshevetskiy@genexis.eu> wrote: > >> 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. > > Do you mean using the sandbox emulator? Yes I think that would be very helpful. > > > > Regards, > > Simon > > > > > >> Mikhail Kshevetskiy > >> > >> > >> On 17.08.2024 18:58, Simon Glass wrote: > >>> Hi Mikhail, > >>> > >>> On Wed, 14 Aug 2024 at 04:32, Mikhail Kshevetskiy > >>> <mikhail.kshevetskiy@iopsys.eu> wrote: > >>>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > >>>> --- > >>>> net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- > >>>> 1 file changed, 37 insertions(+), 33 deletions(-) > >>>> > >>> Reviewed-by: Simon Glass <sjg@chromium.org> [..] Regards, Simon [1] https://docs.u-boot.org/en/latest/develop/testing.html
This needs an actual description, please add details what and why for this 70 line patch. > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > --- > net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- > 1 file changed, 37 insertions(+), 33 deletions(-) > > 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) { > + 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 >
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) { + 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;
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> --- net/tcp.c | 70 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 33 deletions(-)