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

[Gene Cumm]

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?

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.

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?

-- 
-Gene