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
