netdev - Re: [PATCH v2] tcp: pass back data left in socket after receive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iK=HgD65J7ReBPQd4LLLkOD_B-e9TrrDmStAL1WpdYhmg@mail.gmail.com>
Date:   Thu, 28 Apr 2022 16:46:45 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Neal Cardwell <ncardwell@...gle.com>
Cc:     Jens Axboe <axboe@...nel.dk>, netdev <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH v2] tcp: pass back data left in socket after receive

On Thu, Apr 28, 2022 at 4:41 PM Neal Cardwell <ncardwell@...gle.com> wrote:
>
> On Thu, Apr 28, 2022 at 7:23 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Thu, Apr 28, 2022 at 4:13 PM Jens Axboe <axboe@...nel.dk> wrote:
> > >
> > > This is currently done for CMSG_INQ, add an ability to do so via struct
> > > msghdr as well and have CMSG_INQ use that too. If the caller sets
> > > msghdr->msg_get_inq, then we'll pass back the hint in msghdr->msg_inq.
> > >
> > > Rearrange struct msghdr a bit so we can add this member while shrinking
> > > it at the same time. On a 64-bit build, it was 96 bytes before this
> > > change and 88 bytes afterwards.
> > >
> > > Signed-off-by: Jens Axboe <axboe@...nel.dk>
> > > ---
> >
> >
> > SGTM, thanks.
> >
> > Reviewed-by: Eric Dumazet <edumazet@...gle.com>
>
> The patch seems to add an extra branch or two to the recvmsg() fast
> path even for the common application use case that does not use any of
> these INQ features.
>
> To avoid imposing one of these new extra branches for the common case
> where the INQ features are not used, what do folks think about
> structuring it something like the following:
>
>                if (msg->msg_get_inq) {
>                        msg->msg_inq = tcp_inq_hint(sk);
>                        if (cmsg_flags & TCP_CMSG_INQ)
>                                put_cmsg(msg, SOL_TCP, TCP_CM_INQ,
>                                         sizeof(msg->msg_inq),
>                                         &msg->msg_inq);
>                 }

Sure thing.

Note that the prior test would not take this path anyway (unless
someone requests TS)

if ((cmsg_flags || msg->msg_get_inq) ....