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

Hannes Diethelm <[email protected]> writes:

> Am 23.07.26 um 09:47 schrieb Philippe Gerum:
>> Hannes Diethelm <[email protected]> writes:
>>=20
>>> Am 22.07.26 um 20:36 schrieb Philippe Gerum:
>>>> Philippe Gerum <[email protected]> writes:
>>>>
>>>>> Hannes Diethelm <[email protected]> writes:
>>>>>
>>>>>> Am 22.07.26 um 00:36 schrieb Hannes Diethelm:
>>>>>>> Am 21.07.26 um 22:15 schrieb Philippe Gerum:
>>>>>>>> Hannes Diethelm <[email protected]> writes:
>>>>>>>>
>>>>>>>>> Am 20.07.26 um 16:27 schrieb Philippe Gerum:
>>>>>>>>>
>>>>>>>>> An option would be either to support of oob_ioctl() for
>>>>>>>>> SIOCGARP. Or create something like
>>>>>>>>> evl_net_routeinfo(s, addr) returning flags would make probing obs=
olete.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Not entirely. The issue with solely having SIOCGARP or any probe-o=
nly
>>>>>>>> explicit request is that you would have to pair two syscalls at ea=
ch
>>>>>>>> transmit at least, one to probe for the sender address before poss=
ibly
>>>>>>>> soliciting that peer if absent from the oob cache, another one for
>>>>>>>> sending the message eventually. i.e., for every packet:
>>>>>>>>
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0 ioctl(SIOCGARP)
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 !ATF_COM?=
 -> evl_net_solicit()
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0 oob_sendmsg(..., 0)
>>>>>>> You are right, that is an unneeded syscall as long as you don't keep
>>>>>>> a
>>>>>>> list of clients as before. This can remove the need from calling
>>>>>>> evl_net_solicit() in the case the client is already in ARP and
>>>>>>> permanent but
>>>>>>> this will be the only change and won't help that much.
>>>>>>>
>>>>>>>>
>>>>>>>> OTOH, with the latest attempt to address this issue, we can send a
>>>>>>>> probe-only request by passing MSG_PROBE (EHOSTUNREACH), or a reque=
st
>>>>>>>> that does not attempt to defer transmit to the inband stage on pro=
be
>>>>>>>> failure by passing MSG_DONTWAIT (EWOULDBLOCK). i.e., for every pac=
ket
>>>>>>>>
>>>>>>>> redo:
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0 oob_sendmsg(..., MSG_DONTWAIT)
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EWOULDBLO=
CK?
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 oob_sendmsg(..., MSG_PROBE)
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EHOSTUNREACH?
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 evl_n=
et_solicit()
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto =
redo
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 otherwise assume ENOMEM
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 otherwise=
 all done
>>>>>>>>
>>>>>>>> IOW, using a proper combination of MSG_DONTWAIT and MSG_PROBE in t=
he
>>>>>>>> right sequence would either succeed to send the packet immediately=
 on
>>>>>>>> the first oob_sendmsg() call, otherwise fail on memory shortage or
>>>>>>>> missing route from the oob cache. In the latter case, which could =
only
>>>>>>>> happen once for each new peer under normal circumstances, we could
>>>>>>>> disambiguate the EWOULDBLOCK status using an explicit probe.
>>>>>>>>
>>>>>>>> A better way to do this in a single step unambiguously would requi=
re the
>>>>>>>> addition of another operation flag, like MSG_DONTDEFER, preventing=
 the
>>>>>>>> deferral to inband on failed probe and causing oob_sendmsg() to re=
turn
>>>>>>>> with a specific error code. e.g. something as simple as the follow=
ing
>>>>>>>> would cover all requirements:
>>>>>>>>
>>>>>>>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 oob_sendmsg(..., MSG_DONTDEFER) -=
> EADDRNOTAVAIL on failed probe.
>>>>>>>>
>>>>>>>> Now, we might also piggyback off of MSG_OOB instead of defining yet
>>>>>>>> another operation flag, as a way to say "oob only, don't relay to
>>>>>>>> inband", but I'm still pondering whether this would be nicely witt=
y or
>>>>>>>> utterly confusing..
>>>>>>>>
>>>>>>> Yes, this is a variant. I was not aware that MSG_DONTWAIT ->
>>>>>>> EWOULDBLOCK
>>>>>>> can also mean ENOMEM.
>>>>>>> Now after considering all options, i start to prefer the variant
>>>>>>> before
>>>>>>> this patch, keeping a list of already solicit'ed clients. It is str=
aight
>>>>>>> forward, simple and fast. And you know exactly when to expect an in=
 band
>>>>>>> call. Might be we should just drop this patch instead of adding unn=
ecessary
>>>>>>> complexity for example code?
>>>>>>
>>>>>> BTW: With drop this patch I don't mean the one already in but this o=
ne:
>>>>>> [PATCH] tidbits: net-udp: use MSG_DONTWAIT for server mode
>>>>>> https://lore.kernel.org/xenomai/20260717192541.15829-1-hannes.diethe=
[email protected]/T/#u
>>>>>>
>>>>>
>>>>> Got it, it may be worth waiting for the dust to settle in the core
>>>>> regarding the probing issue before adapting this tidbit anyway. I'm
>>>>> going to consolidate the points we have discussed so far, in order to
>>>>> come up with an implementation that satisfies all the requirements.
>>>> Here is a new proposal that should cover all the cases we discussed
>>>> so
>>>> far (next/v6.12.y-cip-evl-rebase branch). A new flag is introduced,
>>>> namely MSG_STEADY, which tells the core to only accept cache lookups
>>>> that deliver permanent ARP records. This flag can be combined with
>>>> MSG_PROBE and MSG_DONTWAIT for extended semantics. Meanwhile, a
>>>> zero-sized message is still accepted in order to request a mere address
>>>> probe without having to pass any meaningful data. This goes like this:
>>>> Function: probe-only
>>>> 	      oob_sendmsg(s, &msghdr, NULL, MSG_PROBE)
>>>> 	          -> EADDRNOTAVAIL if no peer address found
>>>> Function: probe-only, permanent peer address required
>>>>     	      oob_sendmsg(s, &msghdr, NULL, MSG_PROBE|MSG_STEADY)
>>>> 	          -> EADDRNOTAVAIL if no permanent peer address found
>>>>                        (also upon match of aging ARP record)
>>>> Function: send, no deferral (to inband) allowed
>>>> 	      oob_sendmsg(s, &msghdr, NULL, MSG_DONTWAIT)
>>>> 	          -> EADDRNOTAVAIL if no peer address found
>>>> 	          -> EWOULDBLOCK on memory shortage
>>>> Function: send, no deferral (to inband) allowed, permanent peer
>>>> address required
>>>> 	      oob_sendmsg(s, &msghdr, NULL, MSG_DONTWAIT|MSG_STEADY)
>>>> 	          -> EADDRNOTAVAIL if no permanent peer address found
>>>>                        (also upon match of aging ARP record)
>>>> 	          -> EWOULDBLOCK on memory shortage
>>>> Function: send, permanent peer address required
>>>> 	      oob_sendmsg(s, &msghdr, NULL, MSG_STEADY)
>>>> 	          -> EADDRNOTAVAIL if no permanent peer address found
>>>>                        (also upon match of aging ARP record)
>>>>
>>>
>>> Looks promising. This also means after
>>> oob_sendmsg(..., MSG_DONTWAIT|MSG_STEADY)
>>> no second call oob_sendmsg(..., MSG_PROBE) is needed to handle
>>> -ENOMEM right?
>> Correct.
>>=20
>>>
>>> oob_sendmsg(s, &msghdr, NULL, 0) is the only case with deferral to inba=
nd
>>> as it looks in udp.c:421...423?
>>> if (ret !=3D -EADDRNOTAVAIL ||
>>> 	msg_flags & (MSG_PROBE|MSG_DONTWAIT|MSG_STEADY))
>>> 		return ret;
>>>
>>>
>> Open question: does it still make sense to have MSG_DONTWAIT prevent
>> the
>> deferral to the inband stage, now that MSG_STEADY can be used for this?
>> i.e.:
>>=20
>>>> Function: send, no deferral (to inband) allowed
>>>> 	      oob_sendmsg(s, &msghdr, NULL, MSG_DONTWAIT)
>>>> 	          -> EADDRNOTAVAIL if no peer address found
>>>> 	          -> EWOULDBLOCK on memory shortage
>> Not considering MSG_DONTWAIT (alone) anymore as a way to prevent
>> deferral would make MSG_STEADY the only option for this, tightening the
>> requirement from "any cached destination address" to "permanent
>> destination address". This being said:
>> - If $user wants a sure shot wrt to oob transmission, then a
>> permanent
>>    address is most likely desired, in which case MSG_STEADY should be
>>    given. Otherwise, what would be the point of sending to a peer using
>>    an aging ARP record _and_ being concerned about timeliness and
>>    deterministic behavior in general (therefore denying deferral)?
>> - As it is, the implementation somewhat abuses the dont-wait
>>    semantics. As a matter of fact, inband deferral won't make the caller
>>    wait at all, only the packet will be delayed until the regular
>>    netstack picks it. Meanwhile, EWOULDBLOCK is conventionally paired
>>    with MSG_DONTWAIT, not EADDRNOTAVAIL.
>>=20
>
> MSG_DONTWAIT denying inband was only introduced in 644d55e78e8e, right?
>

Yes, this can still be considered as a tentative, undocumented so far.

> In this case I agree. Either you'd like to stay out of band now and in
> the future -> MSG_STEADY or you don't care -> 0.
>
> Otherwise, it might break already existing applications.

--=20
Philippe.