netdev - Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg

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: <bd81277b-a443-461d-9bad-5688c7d8565d@bytedance.com>
Date: Thu, 1 Aug 2024 11:15:11 -0700
From: Zijian Zhang <zijianzhang@...edance.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, linux-api@...r.kernel.org,
 almasrymina@...gle.com, edumazet@...gle.com, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, dsahern@...nel.org, axboe@...nel.dk,
 shuah@...nel.org, linux-kselftest@...r.kernel.org, cong.wang@...edance.com,
 xiaochun.lu@...edance.com
Subject: Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control
 notification test

On 8/1/24 10:36 AM, Willem de Bruijn wrote:
> On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang <zijianzhang@...edance.com> wrote:
>>>
>>>> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
>>>> +static void add_zcopy_info(struct msghdr *msg)
>>>> +{
>>>> +    struct zc_info *zc_info;
>>>> +    struct cmsghdr *cm;
>>>> +
>>>> +    if (!msg->msg_control)
>>>> +            error(1, errno, "NULL user arg");
>>>
>>> Don't add precondition checks for code entirely under your control.
>>> This is not a user API.
>>>
>>
>> Ack.
>>
>>>> +    cm = (struct cmsghdr *)msg->msg_control;
>>>> +    cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
>>>> +    cm->cmsg_level = SOL_SOCKET;
>>>> +    cm->cmsg_type = SCM_ZC_NOTIFICATION;
>>>> +
>>>> +    zc_info = (struct zc_info *)CMSG_DATA(cm);
>>>> +    zc_info->size = ZC_NOTIFICATION_MAX;
>>>> +
>>>> +    added_zcopy_info = true;
>>>
>>> Just initialize every time? Is this here to reuse the same msg_control
>>> as long as metadata is returned?
>>>
>>
>> Yes, the same msg_control will be reused.
>>
>> The overall paradiagm is,
>> start:
>>     sendmsg(..)
>>     sendmsg(..)
>>     ...          sends_since_notify sendmsgs in total
>>
>>     add_zcopy_info(..)
>>     sendmsg(.., msg_control)
>>     do_recv_completions_sendmsg(..)
>>     goto start;
>>
>> if (sends_since_notify + 1 >= cfg_notification_limit), add_zcopy_info
>> will be invoked, and the right next sendmsg will have the msg_control
>> passed in.
>>
>> If (added_zcopy_info), do_recv_completions_sendmsg will be invoked,
>> and added_zcopy_info will be set to false in it.
> 
> This does not seem like it would need a global variable?
> 

Agreed, maybe I can use sends_since_notify to check whether we
need to do_recv_completions_sendmsg, then we get rid of
added_zcopy_info.


>> Btw, before I put some efforts to solve the current issues, I think
>> I should wait for comments about api change from linux-api@...r.kernel.org?
> 
> I'm not sure whether anyone on that list will give feedback.
> 
> I would continue with revisions at a normal schedule, as long as that
> stays in the Cc.

Got it, thanks