[syslinux] [PATCH 0/1] efi: DNS resolver

[Celelibi]

2015-09-28 12:50 UTC+02:00, Gene Cumm <gene.cumm at gmail.com>:
> On Fri, Sep 25, 2015 at 6:40 PM, Celelibi via Syslinux
> <syslinux at zytor.com> wrote:
>> 2015-09-25 21:27 UTC+02:00, Geert Stappers via Syslinux
>> <syslinux at zytor.com>:
>>> On Tue, Sep 15, 2015 at 05:22:40AM -0400, Gene Cumm via Syslinux wrote:
>>>> On Sep 10, 2015 1:32 AM, "celelibi--- via Syslinux"
>>>> <syslinux at zytor.com>
>>>> wrote:
>>>> >
>>>> > From: Sylvain Gault <sylvain.gault at gmail.com>
>>>> >
>>>> > Despite having native network capabilities, UEFI 2.4 (the most
>>>> > widely deployed at the moment) has no native DNS resolver. I propose
>>>> > here an implementation more or less inspired by the one found in
>>>> > core/legacynet/dnsresolv.c.
>>>> >
>>>> > Since it's non-trivial, I'd like to ask for a deep review of this
>>>> > code. I tried to make it as strong as possible against malformed
>>>> > packets and should be devoided of remote code execution or infinite
>>>> > loops (contrary to the legacynet implementation), but I may have
>>>> > forgotten something. I would take any comment,
>>>> > even on the coding style. :)
>>>> >
>>>> > I made a few tests, it worked correctly with a normal dns
>>>> > server. Didn't test malformed packets.
>>>> >
>>>> > Here are the two main missing features of this patch.
>>>> > - No caching implemented.
>>>> > - No timeout management. The efi implementation of core_udp_recv
>>>> >   wait 15ms before giving up.
>>>> >
>>>> > I'd also like to ask if it wouldn't be better to just write an
>>>> > efi driver for lwip and just inherit all the network protocols
>>>> > implemented there, instead of trying to use the buggy native UEFI
>>>> > implementations. Depending on this, I may or may not spend time on
>>>> > this DNS resolver.
>>>> >
>>>> > Sylvain Gault (1):
>>>> >   efi: Implemented a DNS stub resolver for efi
>>>> >
>>>> >  efi/dns.c | 529
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> >  efi/pxe.c |  16 -- 2 files changed, 529 insertions(+), 16
>>>> >  deletions(-) create mode 100644 efi/dns.c
>>>>
>>>> Extending like this to reestablish equality between architectures
>>>> definitely is a good idea.
>>>>
>>>
>>> How to read 'a good idea' ?   Does it mean "patch accepted"???
>>>
>>
>> In any case, I may submit another version (or additional patches if
>> this one get merged) with a few improvements, like removing the
>> recursion limit and retry the same servers several times as the UDP
>> packets may get lost. And coding style improvement.
>
> As you noted, you don't accommodate timeouts which is probably the
> most critical operation this doesn't have.  We can't stay in the query
> forever.  jiffies() and ms_timer() are the two available in the core.
> core/legacynet/dnsresolv.c used jiffies() but ms_timer() should be
> dealing with millisecond values, not 55ms jiffies and about CLK_TCK
> jiffies per second.

Not sure I understand everything correctly.
Currently, the code won't stay in the query forever. It tries every
server once and exit. I think trying every server 3 times would be
better as there may be a single DNS server and UDP packets may get
lost. The only way we can stay in the query forever is if we get
flooded with UDP packets not for us.

I didn't implement a timeout because I didn't want to make a busy
loop. But since EFI is mono-threaded, I think we don't care.

>
> An alternative way to just stub it is to merge everything but the
> actual call that starts sending UDP packets or just the most basic
> functionality you've implemented first.

Not sure I see the point of merging something that wouldn't work.


Celelibi