Re: [PATCH] dns: realloc address buffer after each packet

[Josef Bacik <jbacik@fb.com>]

On 03/01/2016 02:22 PM, Andrei Borzenkov wrote:
> 01.03.2016 19:38, Josef Bacik пишет:
>> On 02/27/2016 12:39 PM, Andrei Borzenkov wrote:
>>> 26.02.2016 16:52, Josef Bacik пишет:
>>>> On 02/26/2016 05:22 AM, Andrei Borzenkov wrote:
>>>>> On Wed, Feb 24, 2016 at 10:11 PM, Josef Bacik <jbacik@fb.com> wrote:
>>>>>> Sometimes DNS responses come in slower than we poll for them which
>>>>>> can lead us
>>>>>> to process multiple DNS packets which overflows the addresses array.
>>>>>> So instead
>>>>>> realloc the array each time to make sure we are accounting for any
>>>>>> answers we
>>>>>> currently have in the address array.  We also move the caching of the
>>>>>> addresses
>>>>>> outside of the recv hook so we can be sure to cache all the responses
>>>>>> at once
>>>>>> instead of one packet at a time.  Thanks,
>>>>>>
>>>>>
>>>>> This still does not address the problem that we stop waiting for
>>>>> further packets as soon as we get any response, so we still depend on
>>>>> delivery order to get correct record.
>>>>>
>>>>> What about following
>>>>>
>>>>> - send both A and AAAA query concurrently if requested
>>>>> - keep track of both requests (i.e. have data.id[2] and
>>>>> data.addresses[2])
>>>>> - reset request ID (or otherwise mark it as "received") as soon as we
>>>>> got reply. Note that reply may contain no addresses - NXDOMAIN is
>>>>> prerfectly valid - so condition should not be "got any record of type
>>>>> A or AAAA" as it is now but rather simply "got reply to request with
>>>>> id XX". This also allows us to implement negative caching at some
>>>>> point :)
>>>>
>>>> So we check the rcode so a NXDOMAIN response will just be discarded, we
>>>> don't have to worry about this case.
>>>>
>>>>> - return both A and AAAA results separately to grub_net_dns_lookup()
>>>>> - combine them in grub_net_dns_lookup() depending on preference - i.e.
>>>>> put either A or AAAA first in final result.
>>>>>
>>>>> This seems to cover all issues so far - we do not wait too long, we
>>>>> are guaranteed to get both A and AAAA if we request them and we return
>>>>> them in proper order for further processing.
>>>>>
>>>>> Do I miss something?
>>>>>
>>>>
>>>> So my patch previous to this one changes it so DNS servers we get from
>>>> dhcp are bound to either ipv4 or ipv6, so the only way we get
>>>> PREFER_IPV* is if an admin sets it.
>>>
>>> In this case I do not follow why you want to collect multiple answers in
>>> the first place. The only reason for me was to make sure we have both A
>>> and AAAA replies; otherwise every reply packet is supposed to contain
>>> exactly the same content. Following this logic the right thing to do is
>>> to just drop all subsequent duplicated replies.
>>
>> I don't want to collect multiple answers.  We bail as soon as we get an
>> answer we care about.
>
> No, we do not. In DNS receive hook we set stop condition and bail out
> when grub_net_poll_cards() checks it. But it checks it only after going
> through all active cards. Multiple DNS servers may be reachable through
> different cards so we can have multiple packets from different servers
> in the same iteration, before stop condition is checked. And even with
> single active card we may still get multiple packets even from single
> server in case replies were delayed because receive_packets() consumes
> multiple packets if present.
>

Sigh sorry I had my old patch in my head where I a) didn't add things to 
the cache if we had a preference and b) stopped once we set data->stop. 
  I'll fix this up.  Thanks,

Josef