On Mon, Jul 27, 2020 at 08:19:24AM -0700, Eric Dumazet wrote:
> On Mon, Jul 27, 2020 at 12:51 AM Jonathan Lemon
> <jonathan.le...@gmail.com> wrote:
> >
> > From: Jonathan Lemon <b...@fb.com>
> >
> > The "header splitting" feature used by netgpu doesn't actually parse
> > the incoming packet header. Instead, it splits the packet at a fixed
> > offset. In order for this to work, the sender needs to send packets
> > with a fixed header size.
> >
> > Signed-off-by: Jonathan Lemon <jonathan.le...@gmail.com>
> > ---
> > net/ipv4/tcp_output.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index d8f16f6a9b02..e8a74d0f7ad2 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -438,6 +438,7 @@ struct tcp_out_options {
> > u8 ws; /* window scale, 0 to disable */
> > u8 num_sack_blocks; /* number of SACK blocks to include */
> > u8 hash_size; /* bytes in hash_location */
> > + u8 pad_size; /* additional nops for padding */
> > __u8 *hash_location; /* temporary pointer, overloaded */
> > __u32 tsval, tsecr; /* need to include OPTION_TS */
> > struct tcp_fastopen_cookie *fastopen_cookie; /* Fast open cookie
> > */
> > @@ -562,6 +563,17 @@ static void tcp_options_write(__be32 *ptr, struct
> > tcp_sock *tp,
> > smc_options_write(ptr, &options);
> >
> > mptcp_options_write(ptr, opts);
> > +
> > +#if IS_ENABLED(CONFIG_NETGPU)
> > + /* pad out options */
> > + if (opts->pad_size) {
> > + int len = opts->pad_size;
> > + u8 *p = (u8 *)ptr;
> > +
> > + while (len--)
> > + *p++ = TCPOPT_NOP;
> > + }
> > +#endif
> > }
> >
> > static void smc_set_option(const struct tcp_sock *tp,
> > @@ -826,6 +838,14 @@ static unsigned int tcp_established_options(struct
> > sock *sk, struct sk_buff *skb
> > opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
> > }
> >
> > +#if IS_ENABLED(CONFIG_NETGPU)
> > + /* force padding */
> > + if (size < 20) {
> > + opts->pad_size = 20 - size;
> > + size += opts->pad_size;
> > + }
> > +#endif
> > +
>
> This is obviously wrong, as any kernel compiled with CONFIG_NETGPU
> will fail all packetdrill tests suite.
>
> Also the fixed 20 value is not pretty.
Would changing this into a sysctl be a suitable solution? It really is
a temporary solution to handle hardware that doesn't support splitting,
and adding a sysctl seems so permanent.....
--
Jonathan
