[syslinux] [PATCH] efi: reuse UDP port with sendto

[Celelibi]

2013/11/30, Gene Cumm <gene.cumm at gmail.com>:
> On Thu, Nov 28, 2013 at 10:24 PM, Celelibi <celelibi at gmail.com> wrote:
>> 2013/11/29, Gene Cumm <gene.cumm at gmail.com>:
>>> On Thu, Nov 28, 2013 at 9:47 PM, Gene Cumm <gene.cumm at gmail.com> wrote:
>>>> On Thu, Nov 28, 2013 at 9:34 PM, Celelibi <celelibi at gmail.com> wrote:
>>>>> Without an assigned source port, Transmit function assign a random new
>>>>> source port to the packet being sent. It thus have to be set before
>>>>> calling Transmit if the source port have already been decided.
>>>>> Conversly, we have to save the assigned port to reuse it later if
>>>>> needed.
>>>>>
>>>>> Resolve bug #35.
>>>>>
>>>>> Signed-off-by: Celelibi <celelibi at gmail.com>
>>>>> ---
>>>>>  efi/udp.c | 18 ++++++++++++++++++
>>>>>  1 file changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/efi/udp.c b/efi/udp.c
>>>>> index 59bb426..7271f1f 100644
>>>>> --- a/efi/udp.c
>>>>> +++ b/efi/udp.c
>>>>> @@ -336,6 +336,9 @@ void core_udp_sendto(struct pxe_pvt_inode *socket,
>>>>> const void *data,
>>>>>
>>>>>      memset(&udata, 0, sizeof(udata));
>>>>>
>>>>> +    /* Re-use the existing local port number if any */
>>>>> +    udata.StationPort = socket->net.efi.localport;
>>>>> +
>>>>
>>>> As HPA noted, this shouldn't be saved here.  Commit 7ec052b on my
>>>> branch efi-fixes (will be renamed to efi-fixes-for-mfleming once Jason
>>>> gives more feedback).
>>>
>>> To clarify, this code-quote reuses (needed) while the removed one
>>> saves and should be in core_udp_open().  Perhaps a fallback-save in
>>> here if the local port is somehow still NULL.
>>>
>>
>> If I understand the behavior of UEFI UDP network stack, the local port
>> choice is made by the firmware only on a call to Transmit. Thus
>> core_udp_open can't chose the local port, or if it does, it's a
>> "manual" choice, not a choice from the firmware itself.
>>
>> As I proposed in the discussion with Jason, the only ways to solve
>> this bug is either to wait for a call to Transmit to chose a port, or
>> to chose a port "by hand" in core_udp_open.
>> This patch I propose implements the first solution.
>
> Celelibi, Jason: Please see also my request at the bottom.
>
> As far as our TFTP traffic from a programming perspective, we need to
> get a fixed StationPort, regardless of StationAddress or any routing
> rules, send a packet to 1 RemotePort, receive a packet from another
> RemotePort then begin a dialog with the second RemotePort and not see
> other traffic.  Would you agree?

That's basically how TFTP works.

>
> Outside UEFI, an open() call would typically assign a port to the
> calling program.  When no port is specified, it shall be a "randomly"
> assigned port (often just the next available local port from the
> "random" pool).  The questions would be 1) What's the spec? and 2) How
> closely do the three or more implementations we've observed follow the
> spec?
>
> UEFI_2.4_0.pdf (page references are print-labeled page numbers not PDF
> page numbers; function context also provided)
>
> p1377 (EFI_UDP4_PROTOCOL.Configure()) "With different parameters in
> UdpConfigData, Configure() can be used to bind this instance to
> specified port." covers the manual case.

Yes, but it doesn't say it will chose a local port if it is not
specified. And that's the whole point. Actually edk2 does, and my real
hardware does too. But I see no guaranty that this behavior would hold
for every compliant implementation. The only guaranty I see is that a
local port will be chosen at least when Transmit is called. And
actually, if AcceptAnyPort is TRUE, only a call to Transmit will
allocate a port in edk2.

>
> p1376 (EFI_UDP4_PROTOCOL.GetModeData()) "StationPort" "The port number
> to which this EFI UDPv4 Protocol instance is bound. If a client of the
> EFI UDPv4 Protocol does not care about the port number, set
> StationPort to zero. The EFI UDPv4 Protocol driver will assign a
> random port number to transmitted UDP packets. Ignored if
> AcceptAnyPort is set to TRUE."
>
> p1385 (EFI_UDP4_PROTOCOL.Transmit()) "SourcePort" "Port from which
> this packet is sent. It is in host byte order. If this field is set to
> zero when sending packets, the port that is assigned in
> EFI_UDP4_PROTOCOL.Configure() is used. If this field is set to zero
> and unbound, a call to EFI_UDP4_PROTOCOL.Transmit() will fail."
>
> So, if AcceptAnyPort == TRUE, the call to Configure() from
> core_udp_open() won't request a port number from the EFI firmware.  As
> long as StationPort remains zero, each call to Transit() may (and
> probably should) result in a new source port.  We need to keep
> RemotePort == 0 until we know the second RemotePort then use
> core_udp_connect() to use that RemotePort exclusively.  I'd say the
> observed behavior should be expected and that it's bug(s) in how
> Syslinux treats EFI.
>
>
>
> Celelibi, Jason:
>
> Is there any chance you two could try my branch efi-fixes (commit
> d04b0edf) as I believe this solves the issues of the changing
> StationPort and routing difficulties of both a single subnet and
> multiple subnets?

I have only one subnet, but I confirm your branch resolves the local
port increment issue.


Celelibi