Message ID | 20240909222720.2563118-9-mikhail.kshevetskiy@iopsys.eu |
---|---|
State | Superseded |
Delegated to: | Ramon Fried |
Headers | show |
Series | net: tcp: improve tcp support | expand |
Hi Mikhail, On Mon, 9 Sept 2024 at 16:27, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote: > > This patch: > * remove useless code, > * use a special function for pretty printing of tcp flags, > * simplify the code > > The behavior should not be changed. > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > --- > net/tcp.c | 74 ++++++++++++++++++++++++++++++------------------------- > 1 file changed, 41 insertions(+), 33 deletions(-) > > diff --git a/net/tcp.c b/net/tcp.c > index 37dda6f84f9..0778c153aca 100644 > --- a/net/tcp.c > +++ b/net/tcp.c > @@ -535,10 +535,41 @@ 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, len; > + char *orig = buf; > + const struct { static > + 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'; > + for (i = 0; desc[i].name; i++) { You could use ARRAY_SIZE() here and avoid the terminator, if you like > + if (!(tcpflags & desc[i].bit)) > + continue; > + > + len = strlen(desc[i].name); > + if (size <= len + 1) > + break; > + if (buf != orig) { > + *buf++ = ','; > + size--; > + } > + > + strcpy(buf, desc[i].name); Or be lazy and just use strlcat() for both the command the name...up to you > + buf += len; > + size -= len; > + } blank line before final return > + 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; > @@ -548,55 +579,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.45.2 > Regards, Simon
diff --git a/net/tcp.c b/net/tcp.c index 37dda6f84f9..0778c153aca 100644 --- a/net/tcp.c +++ b/net/tcp.c @@ -535,10 +535,41 @@ 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, 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'; + for (i = 0; desc[i].name; i++) { + if (!(tcpflags & desc[i].bit)) + continue; + + 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; @@ -548,55 +579,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;
This patch: * remove useless code, * use a special function for pretty printing of tcp flags, * simplify the code The behavior should not be changed. Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> --- net/tcp.c | 74 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 33 deletions(-)