Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control notification test

[Zijian Zhang <zijianzhang@bytedance.com>]

On 8/1/24 10:36 AM, Willem de Bruijn wrote:
> On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang <zijianzhang@bytedance.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@vger.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