Syslinux - May 2017 - [PATCH 3/4] Removing the udp_reader efi_binding

On Wed, May 31, 2017 at 3:56 AM, Julien Viard de Galbert via Syslinux
<syslinux at zytor.com> wrote:
> This extra socket cause the EFI udp stack to duplicate every packet
> to be able to provide them to both udp_reader and
socket->net.efi.binding.
> The ones in socket->net.efi.binding were never read so they accumulated
in
> the stack on EFI side making the stack slower and slower.
> Slow enough to not be able to download a kernel+initramfs before the EFI
> timeout (5 minutes).
>
> Signed-off-by: Julien Viard de Galbert <jviarddegalbert at
online.net>
> ---
>  efi/udp.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)
That almost certainly explains the decaying download rates.
>  extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol;
>
> -/*
> - * This UDP binding is configured to operate in promiscuous mode. It is
> - * only used for reading packets. It has no associated state unlike
> - * socket->net.efi.binding, which has a remote IP address and port
> - * number.
> - */
This part has me puzzled why Matt was creating a promiscuous binding
instead of re-using the existing binding.

-- 
-Gene

Hello,

----- Mail original -----
> De: "Gene Cumm" <gene.cumm at gmail.com>
> ?: "Julien Viard de Galbert" <jviarddegalbert at
online.net>, "Matt Fleming" <matt at console-pimps.org>
> Cc: "For discussion of Syslinux and tftp-hpa" <syslinux at
zytor.com>
> Envoy?: Mercredi 31 Mai 2017 13:45:00
> Objet: Re: [syslinux] [PATCH 3/4] Removing the udp_reader efi_binding
> On Wed, May 31, 2017 at 3:56 AM, Julien Viard de Galbert via Syslinux
> <syslinux at zytor.com> wrote:
>> This extra socket cause the EFI udp stack to duplicate every packet
>> to be able to provide them to both udp_reader and
socket->net.efi.binding.
>> The ones in socket->net.efi.binding were never read so they
accumulated in
>> the stack on EFI side making the stack slower and slower.
>> Slow enough to not be able to download a kernel+initramfs before the
EFI
>> timeout (5 minutes).
>>
>> Signed-off-by: Julien Viard de Galbert <jviarddegalbert at
online.net>
>> ---
>>  efi/udp.c | 24 ++----------------------
>>  1 file changed, 2 insertions(+), 22 deletions(-)
> 
> That almost certainly explains the decaying download rates.
> 
Yes I tracked this from the EFI side (tianocore) until I found that the
datagram where duplicated in the stack this caused most of the delay in
the udp receive function... Only then I could come up with this patch :)
>>  extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol;
>>
>> -/*
>> - * This UDP binding is configured to operate in promiscuous mode. It
is
>> - * only used for reading packets. It has no associated state unlike
>> - * socket->net.efi.binding, which has a remote IP address and port
>> - * number.
>> - */
> 
> This part has me puzzled why Matt was creating a promiscuous binding
> instead of re-using the existing binding.
> 
Yes, to me it looks like some remaining code from a different trial were 
promiscuous mode was needed...
> --
> -Gene
Best Regards,

Julien VdG

On Wed, 31 May, at 07:45:00AM, Gene Cumm wrote:
> >  extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol;
> >
> > -/*
> > - * This UDP binding is configured to operate in promiscuous mode. It
is
> > - * only used for reading packets. It has no associated state unlike
> > - * socket->net.efi.binding, which has a remote IP address and port
> > - * number.
> > - */
> 
> This part has me puzzled why Matt was creating a promiscuous binding
> instead of re-using the existing binding.
Sorry, I'm afraid I don't remember what I was thinking when I wrote
that, so I definitely have no problem with it being ripped out.

On Wed, May 31, 2017 at 10:06 AM, Matt Fleming <matt at
codeblueprint.co.uk> wrote:
> On Wed, 31 May, at 07:45:00AM, Gene Cumm wrote:
>> >  extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol;
>> >
>> > -/*
>> > - * This UDP binding is configured to operate in promiscuous mode.
It is
>> > - * only used for reading packets. It has no associated state
unlike
>> > - * socket->net.efi.binding, which has a remote IP address and
port
>> > - * number.
>> > - */
>>
>> This part has me puzzled why Matt was creating a promiscuous binding
>> instead of re-using the existing binding.
>
> Sorry, I'm afraid I don't remember what I was thinking when I wrote
> that, so I definitely have no problem with it being ripped out.
Gotta love git.  git blame has me thinking it's leftovers from
fe283b78c that weren't cleaned up in c5c37402

It sure looks like it's needed.

-- 
-Gene