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

[Josef Bacik]

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 <address@hidden> 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