Guidelines about fixing bugs

[Uri Simchoni]

On 02/03/2017 01:46 AM, Matthieu Patou wrote:
> On 02/02/2017 12:15 PM, Uri Simchoni wrote:
<snip>
>> - My general idea is to turn the API from one that returns an IP list
>> into one that returns an iterator-style, opaque object that you can
>> extract addresses from until you run out of addresses. In the
>> implementation, the A queries will thus be done lazily and only if
>> needed (keep leveraging the SRV query additional records). As we make A
>> queries we filter answers that have been provided by previous queries.
> We could change struct ip_service to have the name in addition to
> sockaddr_storage, so that if the sockaddr_storage is not populated when
> we need it we could still populate it.

Haven't looked carefully, but it seems like it could work. Seems like it
would make the interface less easy to use (have to call again into the
name resolution code, "leaky abstraction"), but sometimes a simple
implementation wins over a cleaner interface.

>> - Won't be 100% compatible - won't be able to support the current method
>> of spraying CLDAPs simultaneously to all DCs to see who responds first.
>> We can work in groups of N each time.
> That's the major challenge, I haven't looked at all the code but just
> looking at get_kdc_ip_string we first lookup all the IPs, then create
> all the tsocket and then start to spray CLDAPs.
> All the code that is dealing with results from internal_resolve_name
> needs to be changed to deal with the lazy loading of records.

I don't think we need to spray CLDAPs to *all* DC's - we can spray to
up-to-N DCs. This "spraying" only occurs when findsing KDC's, for the
sake of building a custom krb5.conf. We try to come up with up to 3
KDCs. So we could "spray", say, 5 CLDAPs, hopefully getting at least 3
replies, then if we don't have enough, try more servers out of the list.
The advantage of sending CLDAP to all servers is that you get the
fastest-responding ones, but it doesn't scale. Hopefully all DCs on-site
respond fast enough.

In other cases (finding a DC for LDAP or SMB) we try them one-by-one.

>> - Should support returning next N addresses instead of next 1. That
>> would allow us to later enhance the implementation to leverage the async
>> geteaddrinfo_send() API.
> I'm not sure I get this point.

That's just a requirement for a new API (not internal_resolve_name),
with get-next semantics - it should be able to get-next-N, so the
implementation can utilize async. If instead of new API you go for
returning the name and resolving it later, this point is irrelevant.