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] [day] [month] [year] [list]

Message-ID: <3d6f12c3-48d0-d7ff-6228-c2f4d04e8c7b@kernel.dk>
Date:   Thu, 28 Apr 2022 18:40:20 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Neal Cardwell <ncardwell@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH v2] tcp: pass back data left in socket after receive

On 4/28/22 5:41 PM, Neal Cardwell 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);
>                 }

I'm fine with that, doesn't really matter to me. You're under that
cmsg_flags branch anyway.

-- 
Jens Axboe