Re: squid-3.HEAD IPAddress leak

On Fri, 2008-02-08 at 11:46 +1300, Amos Jeffries wrote:

> The basics:
>
> addrinfo* extends the neutral sockaddr* type ( union { sockaddr_in,
> sockaddr_in6, sockaddr_storage} ) while adding the size values and
> moving the flags and protocol details.

Hi Amos,

    I understand that we need to support IPv4 and IPv6 addresses. I do
not understand why we are suddenly talking about pointers and trees?

I can understand if wherever Squid2 was using an IPv4 address, Squid3
would use an (ip4, ip6, whichOneFlag) class with appropriate methods
that check the flag and return or set the right thing. I know that would
not be the best way to implement it, but it would work and I would
understand what is going on.

Can you explain AddrInfo in the above simple (ip4, ip6, whichOneFlag)
class terms? Why do we need AddrInfo now and why are we talking about a
"tree" with "pointers" and alloc/free operations?

Did Squid2 use an equivalent of AddrInfo? If yes, did it use it as often
as Squid3 does now? Did Squid2 allocated and freed it?

Thank you,

Alex.

> I'm using it in three ways.
>
> First Usage: [simplest via InitAddrInfo() paired with an assignment
> somewhere ]
> - to create an empty addrinfo tree root and pass to the system to be
> filled out. The results are copied to the fde or xxData objects for later
> async use.
>
>
> Second usage: [ hiding complex setup via GetAddrInfo() ]
> - to create addrinfo tree containing the current IPAddress so system or
> squid can use the structure fields in some comm call.
> Variant 1: addrinfo provides: AF_*, sockaddr_storage*, and sizeof()
> results in one package with fixed field names/locations for squid to
> send individually to a system call without mode-specific #ifdef's.
> Variant 2: whole addrinfo goes to system for use.
>
>
> Third Usage: [ hiding complex usage via GetAddrInfo() paired with an
> assignment somewhere ]
> - to create addrinfo tree containing the current IP and socket
> information so system can update/add its knowledge of the FQDN,
> hostname, alternate IPs, socket options, or original destination (in NAT
> case).
> (both above Variantions apply here too)
> - which then gets saved somewhere for squid to use later.
>
>
> All three uses above end up with a tree of dynamic data squid is expected
> to cleanup. So all uses are terminated with a FreeAddrInfo() to perform
> that cleanup.
>
> >
> > There's a netural sock addr size type - its called sockaddr_storage.
> > addr_info is for return results from hostname<->ip queries..
> >
>
> struct addrinfo extends struct sockaddr_storage to handle multiple IPs,
> hostnames, and removes the need for many type-castings in user-space code.
>
> Amos
>
> >
> > Adrian
> >
> >
> > On Fri, Feb 08, 2008, Amos Jeffries wrote:
> >> > On Thu, Feb 07, 2008, Robert Collins wrote:
> >> >
> >> >> > Still, this is one of those "death of a thousand cuts" method of
> >> >> > killing performance..
> >> >>
> >> >> Right, I haven't seen the commit; care to mail the diff?
> >> >
> >> > Which? I just looked at the places where Amos is calling GetAddrInfo()
> >> > and FreeAddrInfo(); more then one are:
> >> >
> >> > * GetAddrInfo(temp, );
> >> > * F->{something} = temp;
> >> > * FreeAddrInfo(temp);
> >>
> >> Where?!
> >>
> >> I hate addrinfo enough that I only added addrinfo where it needed:
> >>
> >> GetAddrInfo(temp)
> >> syscall_needing_neutral_sockaddr_size_family(temp)
> >> FreeAddrInfo(temp)
> >>
> >> OR:
> >>
> >> GetAddrInfo(temp)
> >> syscall_altering_addrinfo(temp)
> >> F->something = temp;
> >> FreeAddrInfo(temp)
> >>
> >>
> >> Amos
> >>
> >
> > --
> > - Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid
> > Support -
> > - $25/pm entry-level VPSes w/ capped bandwidth charges available in WA -
> >
>
Received on Thu Feb 07 2008 - 17:40:10 MST