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

[Dmitrii Merkurev]

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


Got rid of all macro conditions across the whole patch stack. Tested it on
different configurations. Should be ok.

Please add a full comment


Added

On Sun, Jul 23, 2023 at 4:49 AM Simon Glass <sjg at chromium.org> wrote:

> 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
>