[PATCH v2 3/6] net: introduce TCP/IP6 support

[Simon Glass]

Hi Dmitrii,

On Wed, 10 May 2023 at 11:00, Dmitrii Merkurev <dimorinny at google.com> wrote:
>
> Add TCP/IP6 related headers and reuse refactored TCP/IP
> implementation
>
> Signed-off-by: Dmitrii Merkurev <dimorinny at google.com>
> Cc: Ying-Chun Liu (PaulLiu) <paul.liu at linaro.org>
> Cc: Simon Glass <sjg at chromium.org>
> Сс: Joe Hershberger <joe.hershberger at ni.com>
> Сс: Ramon Fried <rfried.dev at gmail.com>
> ---
>  include/net/tcp6.h | 106 +++++++++++++++++++++++++++++++++++++++++++++
>  include/net6.h     |  14 ++++++
>  net/Makefile       |   1 +
>  net/net.c          |   6 +++
>  net/net6.c         |  72 +++++++++++++++++++++++++-----
>  net/tcp6.c         |  99 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 288 insertions(+), 10 deletions(-)
>  create mode 100644 include/net/tcp6.h
>  create mode 100644 net/tcp6.c

Reviewed-by: Simon Glass <sjg at chromium.org>

Comments below.

>
> diff --git a/include/net/tcp6.h b/include/net/tcp6.h
> new file mode 100644
> index 0000000000..3db125ecc5
> --- /dev/null
> +++ b/include/net/tcp6.h
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2022 The Android Open Source Project
> + */
> +
> +#ifndef __TCP6_H__
> +#define __TCP6_H__
> +
> +#if defined(CONFIG_PROT_TCP)

Is there anything that can be done to get rid of these, at least in
the source code?

> +
> +#include <net6.h>
> +#include <net/tcp.h>
> +
> +/**
> + * typedef rxhand_tcp6_f() - An incoming TCP IPv6 packet handler.
> + * @pkt: pointer to the application packet
> + * @dport: destination TCP port
> + * @sip: source IP6 address
> + * @sport: source TCP port
> + * @tcp_seq_num: TCP sequential number
> + * @tcp_ack_num: TCP acknowledgment number
> + * @action: TCP packet type (SYN, ACK, FIN, etc)
> + */
> +typedef void rxhand_tcp6_f(uchar *pkt, u16 dport,
> +                          struct in6_addr sip, u16 sport,
> +                          u32 tcp_seq_num, u32 tcp_ack_num,
> +                          u8 action, unsigned int len);
> +
> +/**
> + * struct ip6_tcp_hdr_o - IP6 + TCP header + TCP options
> + * @ip_hdr: IP6 + TCP header
> + * @tcp_hdr: TCP header
> + * @tcp_o: TCP options
> + * @end: end of IP6/TCP header
> + */
> +struct ip6_tcp_hdr_o {
> +       struct  ip6_hdr    ip_hdr;
> +       struct  tcp_hdr    tcp_hdr;
> +       struct  tcp_hdr_o  tcp_o;
> +       u8      end;
> +} __packed;
> +
> +#define IP6_TCP_O_SIZE (sizeof(struct ip6_tcp_hdr_o))
> +
> +/**
> + * struct ip6_tcp_hdr_s - IP6 + TCP header + TCP options
> + * @ip_hdr: IP6 + TCP header
> + * @tcp_hdr: TCP header
> + * @t_opt: TCP Timestamp Option
> + * @sack_v: TCP SACK Option
> + * @end: end of options
> + */
> +struct ip6_tcp_hdr_s {
> +       struct  ip6_hdr    ip_hdr;
> +       struct  tcp_hdr    tcp_hdr;
> +       struct  tcp_t_opt  t_opt;
> +       struct  tcp_sack_v sack_v;
> +       u8      end;
> +} __packed;
> +
> +#define IP6_TCP_SACK_SIZE (sizeof(struct ip6_tcp_hdr_s))
> +
> +/**
> + * union tcp6_build_pkt - union for building TCP/IP6 packet.
> + * @ip: IP6 and TCP header plus TCP options
> + * @sack: IP6 and TCP header plus SACK options
> + * @raw: buffer
> + */
> +union tcp6_build_pkt {
> +       struct ip6_tcp_hdr_o ip;
> +       struct ip6_tcp_hdr_s sack;
> +       uchar  raw[1600];
> +} __packed;
> +
> +/**
> + * net_set_tcp6_handler6() - set application TCP6 packet handler
> + * @param f pointer to callback function
> + */
> +void net_set_tcp_handler6(rxhand_tcp6_f *f);
> +
> +/**
> + * net_set_tcp_header6() - set
> + * @pkt: pointer to IP6/TCP headers
> + * @dport: destination TCP port
> + * @sport: source TCP port
> + * @payload_len: payload length
> + * @action: TCP packet type (SYN, ACK, FIN, etc)
> + * @tcp_seq_num: TCP sequential number
> + * @tcp_ack_num: TCP acknowledgment number
> + *
> + * returns TCP header size
> + */
> +int net_set_tcp_header6(uchar *pkt, u16 dport, u16 sport, int payload_len,
> +                       u8 action, u32 tcp_seq_num, u32 tcp_ack_num);

Please add a full comment. We need full comments on all exported
functions and any non-trivial internal ones.

> +
> +void net_set_tcp_handler6(rxhand_tcp6_f *f);
> +
> +/**
> + * rxhand_tcp6() - handle incoming IP6 TCP packet
> + * @param b pointer to IP6/TCP packet builder struct
> + * @param len full packet length
> + */
> +void rxhand_tcp6(union tcp6_build_pkt *b, unsigned int len);
> +
> +#endif // CONFIG_PROT_TCP
> +#endif // __TCP6_H__
> diff --git a/include/net6.h b/include/net6.h
> index beafc05338..fa926f07ac 100644
> --- a/include/net6.h
> +++ b/include/net6.h
> @@ -344,6 +344,20 @@ int ip6_add_hdr(uchar *xip, struct in6_addr *src, struct in6_addr *dest,
>  int net_send_udp_packet6(uchar *ether, struct in6_addr *dest, int dport,
>                          int sport, int len);
>
> +/**
> + * net_send_tcp_packet6() - Make up TCP packet and send it
> + *
> + * @payload_len: TCP payload length
> + * @dport:      destination port
> + * @sport:      source port
> + * @action:     TCP flag (SYN, ACL, PUSH, etc)
> + * @tcp_seq_num: TCP sequence number
> + * @tcp_ack_num: TCP ackno
> + * Return: 0 if send successfully, -1 otherwise
> + */
> +int net_send_tcp_packet6(int payload_len, int dport, int sport, u8 action,
> +                        u32 tcp_seq_num, u32 tcp_ack_num);

as you have done here

Regards,
Simon