Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <f9bc6eb8fb9c0771d7fff0b4da1a75528c5d6c07.camel@redhat.com>
Subject: Re: [PATCH net-next 00/17] Change cork to a pointer in sockets
From:   Paolo Abeni <pabeni@redhat.com>
To:     Oliver Crumrine <ozlinuxc@gmail.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     davem@davemloft.net
Date:   Mon, 23 Oct 2023 10:24:31 +0200
In-Reply-To: <cover.1697989543.git.ozlinuxc@gmail.com>
References: <cover.1697989543.git.ozlinuxc@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Sun, 2023-10-22 at 12:19 -0400, Oliver Crumrine wrote:
> This patch changes the cork field of both the inet_sock and inet6_sk
> structs to a pointer, reducing their size.
>=20
> Oliver Crumrine (17):
>   Make cork in inet_sock a pointer.
>   Allocate and free cork in inet_create and inet_release in file
>     net/ipv4/af_inet.c
>   Change cork in ipv6_pinfo to a pointer.
>   Allocate and free cork in inet6_sk.
>   Change occurence of cork in inet_sock to pointer in include/net/ip.h
>   Since cork in inet_sock and inet6_sk will be a pointer, they don't
>     need to be referenced in this function in include/net/ipv6.h
>   Change occurences of cork in inet_sock to pointer in file
>     net/ipv4/af_inet.c
>   Change occurence of cork to pointer in file net/ipv4/datagram.c
>   Change instances of cork in net/ipv4/inet_connection_sock.c to
>     pointer.
>   Change instances of cork to pointer in net/ipv4/ip_output.c
>   Update occurences of cork to pointer in net/ipv4/syncookies.c
>   Change occurences of cork to pointer in net/ipv4/tcp_output.c
>   Change instances of cork to a pointer in file net/ipv4/udp.c
>   Update usages of cork in net/ipv6/ip6_output.c to pointer.
>   Modify occurences of cork in net/ipv6/raw.c to use a pointer
>   Change usages of cork to pointer in net/ipv6/udp.c
>   Change instance of cork to pointer in net/ipv4/tcp_ipv4.c
>=20
>  include/linux/ipv6.h            |  2 +-
>  include/net/inet_sock.h         |  2 +-
>  include/net/ip.h                |  2 +-
>  include/net/ipv6.h              |  4 ++--
>  net/ipv4/af_inet.c              |  6 ++++--
>  net/ipv4/datagram.c             |  2 +-
>  net/ipv4/inet_connection_sock.c |  6 +++---
>  net/ipv4/ip_output.c            |  6 +++---
>  net/ipv4/syncookies.c           |  2 +-
>  net/ipv4/tcp_ipv4.c             |  2 +-
>  net/ipv4/tcp_output.c           |  2 +-
>  net/ipv4/udp.c                  |  8 ++++----
>  net/ipv6/af_inet6.c             |  5 +++++
>  net/ipv6/ip6_output.c           | 10 +++++-----
>  net/ipv6/raw.c                  |  4 ++--
>  net/ipv6/udp.c                  |  4 ++--
>  16 files changed, 37 insertions(+), 30 deletions(-)

Could you please explain the rationale behind such changes?=C2=A0

As the cork struct is still allocated for each inet/inet6 socket, the
total memory used by the the now smaller socket struct and the cork=20
struct will be greater then the memory used originally by such socket.
And the double allocation/free will be slower then the original one.

This also adds a bunch of additional pointer de-reference in the xmit
path.

Finally the above change will probably conflict with the goodies
introduced by:

https://lore.kernel.org/netdev/20231017014716.3944813-1-lixiaoyan@google.co=
m/

I'm sorry, but this looks really a no-go.

Before future submissions, please read thoroughly the process
documentation, including the netdev specific bits in maintainer-
netdev.rst: there a bit of issues with the process here (the recipients
list does not include a lot of relevant ones, there are typos there,
the patch series is too long, it breaks the builds ...)

Cheers,

Paolo