[Bug 296955] ports-mgmt/pkg: pkg only tries one server when using multiple SRV RR or a mirrorlist

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=296955

            Bug ID: 296955
           Summary: ports-mgmt/pkg: pkg only tries one server when using
                    multiple SRV RR or a mirrorlist
           Product: Ports & Packages
           Version: Latest
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: Individual Port(s)
          Assignee: pkg@FreeBSD.org
          Reporter: freebsd@dev.thsi.be
                CC: bapt@FreeBSD.org
          Assignee: pkg@FreeBSD.org
             Flags: maintainer-feedback?(pkg@FreeBSD.org)

Created attachment 273051
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=273051&action=edit
patch-libpkg_fetch__libfetch.c

Hi,

In libpkg/fetch_libfetch.c:libfetch_open()  both the srv_current and http
pointer are reset at the start of the while loop.

The intent seems to try other servers as srv_current is set to
srv_current->next and http_current to http_current->next at the end of the
loop.

The expectation is that multiple SRV RR should be tried until one works, as per
RFC 2782. The sorting/weighting seems to be implemented already.

With the attached patch, failing over successive SRV server works (almost, see
next issue A). And maybe in the case of http mirror list, but I haven't tested
it (I don't know how it works). The patch is against an old ports tree having
pkg still at version 2.7.5, though it is my understanding that this code hasn't
changed much so it should be trivially applicable to pkg's HEAD or onto a newer
ports tree.

Please note that this patch aims to make it clear what the problem is and to
serve as a minimal workaround for my use-case, but it is incomplete because but
it doesn't fix two related issues:

A) the retry count is decremented even when processing the next server in list.
Meaning that having more than 3 SRV records (or http mirrors) is not useful to
support 3 server failures (it would only randomly work, at best, depending on
how the priority/weight are setup). IMHO there should be two kinds of retry:
one for computing the server list (dns_getsrvinfo/gethttpmirrors, DNS can be
unreliable and should be tried with some decaying period), and one per each
server attempt. Trying the next server in the list should (IMO) not count
against a retry count, as it is be perfectly fine to have "unreachable" servers
(as per RFC 2782). Ideally, the code should try each server n-times, then jump
to the next item of the list.

A quick workaround would be to multiply the retry count by the length of the
server list, the first time the list is computed. This would make the current
code roughly go up to n-times over the whole list (modulo differences in
successive list computations).

B) dns_getsrvinfo failures are ignored. In the case that dns_getsrvinfo returns
NULL, then I guess that the code keeps going and tries the original url host
with the port set to 0, which is likely to fail but is undesirable nonetheless.
I think that a proper error message should be issued instead so that users know
that their DNS setup is broken.

Fixing these would require more restructuring of the code which I'm not
comfortable trying. I'm guessing that what is required is to separate errors
arising from the server list computation from errors resulting from per-server
connection attempts. I hope this helps,

Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.