From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aaq0Q-0000ri-SM for mharc-grub-devel@gnu.org; Tue, 01 Mar 2016 14:30:50 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34764) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaq0O-0000pp-Hy for grub-devel@gnu.org; Tue, 01 Mar 2016 14:30:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaq0L-0008Db-OX for grub-devel@gnu.org; Tue, 01 Mar 2016 14:30:48 -0500 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:52575) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaq0L-0008DE-Hf for grub-devel@gnu.org; Tue, 01 Mar 2016 14:30:45 -0500 Received: from pps.filterd (m0001255.ppops.net [127.0.0.1]) by mx0b-00082601.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id u21JSeQb011726; Tue, 1 Mar 2016 11:30:43 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=subject : to : references : cc : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=facebook; bh=cqMvAiDBncYgQusGT3INBgT4vorf6ENTrGhl+1nTyIg=; b=c6BHOrTsOYh2rtiKMdxDpbskgv7gQZxNwAp1FByizcNiv8YHNIjt702axgJrJSTrQ6/p aNzAMpIUxFNncQoxrgwyxOMEF/RfBHvfFtUVwo+0VJk0IFPQIlApf+4/BioNvZRqy290 MU5sWWEPmfLvoCQ/vQ/tTZoWEcR+L4Qe9bM= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 21dgr7068c-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Tue, 01 Mar 2016 11:30:43 -0800 Received: from localhost.localdomain (192.168.52.123) by mail.thefacebook.com (192.168.16.16) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 1 Mar 2016 11:30:37 -0800 Subject: Re: [PATCH] dns: realloc address buffer after each packet To: Andrei Borzenkov , The development of GNU GRUB References: <1456341072-13356-1-git-send-email-jbacik@fb.com> <56D058B3.6060202@fb.com> <56D1DF39.5060005@gmail.com> <56D5C572.3020600@fb.com> <56D5EBFC.7010504@gmail.com> From: Josef Bacik Message-ID: <56D5EDDB.3000809@fb.com> Date: Tue, 1 Mar 2016 14:30:35 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56D5EBFC.7010504@gmail.com> Content-Type: text/plain; charset="utf-8"; format=flowed X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-03-01_09:, , signatures=0 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx0b-00082601.pphosted.com id u21JSeQb011726 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 67.231.153.30 Cc: Kernel Team X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Mar 2016 19:30:50 -0000 On 03/01/2016 02:22 PM, Andrei Borzenkov wrote: > 01.03.2016 19:38, Josef Bacik =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> On 02/27/2016 12:39 PM, Andrei Borzenkov wrote: >>> 26.02.2016 16:52, Josef Bacik =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>> On 02/26/2016 05:22 AM, Andrei Borzenkov wrote: >>>>> On Wed, Feb 24, 2016 at 10:11 PM, Josef Bacik 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 arra= y. >>>>>> 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 respon= ses >>>>>> 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 ty= pe >>>>> A or AAAA" as it is now but rather simply "got reply to request wit= h >>>>> 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 ret= urn >>>>> 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 fr= om >>>> 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 a= n >> 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=20 the cache if we had a preference and b) stopped once we set data->stop.=20 I'll fix this up. Thanks, Josef