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

Hannes Diethelm <[email protected]> writes:

> Am 22.07.26 um 20:36 schrieb Philippe Gerum:
>> Philippe Gerum <[email protected]> writes:
>>=20
>>> 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 obsol=
ete.
>>>>>>>
>>>>>>
>>>>>> Not entirely. The issue with solely having SIOCGARP or any probe-only
>>>>>> explicit request is that you would have to pair two syscalls at each
>>>>>> transmit at least, one to probe for the sender address before possib=
ly
>>>>>> 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 request
>>>>>> that does not attempt to defer transmit to the inband stage on probe
>>>>>> failure by passing MSG_DONTWAIT (EWOULDBLOCK). i.e., for every packet
>>>>>>
>>>>>> 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 EWOULDBLOCK?
>>>>>>  =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_net_=
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 al=
l done
>>>>>>
>>>>>> IOW, using a proper combination of MSG_DONTWAIT and MSG_PROBE in the
>>>>>> 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 on=
ly
>>>>>> 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 require=
 the
>>>>>> addition of another operation flag, like MSG_DONTDEFER, preventing t=
he
>>>>>> deferral to inband on failed probe and causing oob_sendmsg() to retu=
rn
>>>>>> with a specific error code. e.g. something as simple as the following
>>>>>> would cover all requirements:
>>>>>>
>>>>>>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 oob_sendmsg(..., MSG_DONTDEFER) -> E=
ADDRNOTAVAIL 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 witty =
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 strai=
ght
>>>>> forward, simple and fast. And you know exactly when to expect an in b=
and
>>>>> call. Might be we should just drop this patch instead of adding unnec=
essary
>>>>> complexity for example code?
>>>>
>>>> BTW: With drop this patch I don't mean the one already in but this one:
>>>> [PATCH] tidbits: net-udp: use MSG_DONTWAIT for server mode
>>>> https://lore.kernel.org/xenomai/20260717192541.15829-1-hannes.diethelm=
@gmail.com/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)
>>=20
>
> 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.

>
> oob_sendmsg(s, &msghdr, NULL, 0) is the only case with deferral to inband
> 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.:

>> 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.

> I have to test it and will send an update to the oob_net_udp patch
> soon.

Thanks.

>
> BTW: CC [email protected] got lost in my last email. I added it aga=
in.

--=20
Philippe.