Re: [PATCH 1/1] tidbits: net-udp: solicit new client for server mode

Philippe Gerum <[email protected]> writes:

> Hannes Diethelm <[email protected]> writes:
>
>> Am 20.07.26 um 16:27 schrieb Philippe Gerum:
>>> Hannes Diethelm <[email protected]> writes:
>>> 
>>>> Am 04.07.26 um 19:52 schrieb Philippe Gerum:
>>>>> Hannes Diethelm <[email protected]> writes:
>>>>>
>>>>>> Am 20.06.26 um 19:28 schrieb Philippe Gerum:
>>>>>>> - Honor MSG_PROBE for oob_sendmsg(), so that only the general call
>>>>>>>      sanity and route resolution to the destination host is performed when
>>>>>>>      set in the request flags, without actually sending any data. On
>>>>>>>      success of such call, we would know that the routing information is
>>>>>>>      readily available from the oob caches, no offload to in-band would
>>>>>>>      have happened if we had not given this flag. The absence of routing
>>>>>>>      information to the destination from some oob cache would yield a
>>>>>>>      specific error, so that the caller may decide what to do next.
>>>>>>
>>>>>> It seams the flag MSG_PROBE is kernel only? I did not find any occurrence
>>>>>> in /usr/include or in libevl.
>>>>>>
>>>>> Yep, my bad. Using MSG_PROBE is not the right way, since that would
>>>>> conflict with MSG_PROXY in userland which has a totally different
>>>>> meaning. I have revisited the implementation, simplifying it actually:
>>>>> since the evl netstack already accepts zero-sized messages, sending such
>>>>> a datagram to the UDP layer now amounts to returning early with the
>>>>> address resolution status, short-circuiting the logic before the actual
>>>>> transmission happens.
>>>>> IOW, passing a NULL or empty iov into the msghdr struct does what
>>>>> MSG_PROBE was intended to do.
>>>>
>>>> I tested this variant. It works. But I wonder:
>>>> If I use:
>>>> ret = oob_sendmsg(s, &msghdr, NULL, 0); errno is set to EHOSTUNREACH
>>>> If I use:
>>>> ret = oob_sendmsg(s, &msghdr, NULL, MSG_DONTWAIT); errno is set to EWOULDBLOCK
>>>>
>>>> Is this intended? EHOSTUNREACH is like halve correct. Yes, the host can not be reached
>>>> but only due to no ARP request is sent.
>>>>
>>> EHOSTUNREACH was intended as a way to distinguish from EWOULDBLOCK
>>> wrt
>>> lack of buffer space for the outgoing message, this code was the only
>>> option close enough to the idea to be conveyed available from the errno
>>> list that would not conflict with other situations. Now, since such
>>> probing mode needs no message space in the first place, this is guarding
>>> against the impossible, which does not make sense. Returning
>>> -EWOULDBLOCK in both cases above would still be
>>> practical. e.g. something along these lines:
>>> diff --git a/kernel/evl/net/ipv4/udp.c b/kernel/evl/net/ipv4/udp.c
>>> index d819616c3b5b..5912a1fbcfb5 100644
>>> --- a/kernel/evl/net/ipv4/udp.c
>>> +++ b/kernel/evl/net/ipv4/udp.c
>>> @@ -417,22 +417,22 @@ static ssize_t send_udp(struct evl_socket *esk,
>>>   	 * address.
>>>   	 */
>>>   	ret = find_egress_path(esk, daddr, &ert, &earp, &pseudo_earp, msg_flags);
>>> -	if (ret == -EMULTIHOP)
>>> -		return ret;	/* MSG_DONTROUTE cannot be honored. */
>>> -
>>>   	if (ret) {
>>> +		if (ret != -EHOSTUNREACH)
>>> +			return ret;
>>> +
>>> +		if (datalen == 0)
>>> +			return -EWOULDBLOCK; /* Address probe failed. */
>>> +
>>>   		/*
>>> -		 * No route known from the front cache - bummer. We
>>> -		 * may have to offload the transmit operation to the
>>> -		 * in-band stack, unless only probing or MSG_DONTWAIT
>>> -		 * is set.
>>> +		 * We have a message to send but no route was found in
>>> +		 * the front cache - bummer. We may have to offload
>>> +		 * the transmit operation to the in-band stack, unless
>>> +		 * only probing or MSG_DONTWAIT is set.
>>>   		 */
>>>   		if (msg_flags & MSG_DONTWAIT)
>>>   			return -EWOULDBLOCK;
>>>   -		if (datalen == 0)
>>> -			return ret;
>>> -
>>>   		/*
>>>   		 * We always charge the socket even when offloading to
>>>   		 * the in-band stack although we won't consume any
>>> 
>>
>> I think in this case, it is fine as it is. You also won't expect EWOULDBLOCK or EAGAIN as long as you
>> don't set MSG_DONTWAIT.
>>
>> But now there are two ways of probing. Either with or withouth MSG_DONTWAIT that behave slightly different.
>> Might be just support MSG_DONTWAIT -> EWOULDBLOCK and drop the other
>> variant?
>
> You mean detect a probing request when receiving MSG_DONTWAIT and a
> zero-sized buffer? That is an option. Another option would be to always
> return EHOSTUNREACH/??? on failed probe regardless of whether
> MSG_DONTWAIT is set. I would preferably go for the second option iff we
> can settle on a unconfusing, unambiguous error status.
>

Ok, I believe that the best option is to go back to an explicit
operation flag for probing eventually, because all other options seem
confusing.  Therefore MSG_PROBE handling was resurrected [1] in the udp
layer, and EHOSTUNREACH is unambiguously used to denote a failed probe
if MSG_PROBE is set.

As you pointed out, the MSG_PROBE definition is missing from the common
socket.h bits in user-space, but the same flag value is defined as
MSG_PROXY in *libc headers, which looks like obsolete. Anyway, to
address this, MSG_PROBE was added to the evl/uapi bits with enough
guards to prevent conflicts or mismatches. It defines the constant
expected by the kernel, which is very unlikely to change since this
belongs to the part of the ABI which is written in stone.

The commit log states the possible outcomes depending on the operation
flags and/or size of the message passed to oob_sendmsg().

[1]
https://gitlab.com/Xenomai/xenomai4/linux-evl/-/commit/566f032dc3bc4c3e945e94887b7f57cd6e57dfb6

-- 
Philippe.