Re: Finalizing source IP version

On 09/09/11 02:51, Alex Rousskov wrote:
> On 09/08/2011 08:11 AM, Amos Jeffries wrote:
>> ------------------------------------------------------------
>> revno: 11715
>> fixes bug(s): http://bugs.squid-cache.org/show_bug.cgi?id=3335
>> committer: Amos Jeffries<squid3_at_treenet.co.nz>
>> branch nick: trunk
>> timestamp: Fri 2011-09-09 02:11:05 +1200
>> message:
>> Bug 3335: ICAP service is down
>> modified:
>> src/adaptation/icap/Xaction.cc
>
>> getOutgoingAddress(NULL, connection);
>> + if (connection->remote.IsIPv4()&& !connection->local.SetIPv4()) {
>> + // This should never happen. getOutgoing should match by family or skip.
>> + Must(connection->local.IsAnyAddr());
>> + return;
>> + }
>
>
> Hi Amos,
>
> IIRC, you have mentioned that already, but I want to stress that
> there is something really wrong with our code if the above explicit
> SetIPv4() call is necessary for a high-level module opening a
> connection. There should be virtually no IP-version specific code in
> high-level modules such as ICAP!
>
> Judging by the name of it, it should be getOutgoingAddress()
> responsibility to finalize the local source address. However, if that
> address is not set for whatever reason, the connection establishing code
> in Comm should not just fail but should select the right IP version if a
> specific IP version must be selected at that time.

Yes. I'm following up with a move of it into getOutgoingAddress where I
agree it should be.

Conceptually the change applied and the change to using
getOutgoingAddress() are both right. The code doing "new
Comm::Connection" is responsible for setting the template up correctly.
And the default :: is no longer a valid ANY_ADDR in IPv4 since the
ANY_ADDR change a few weeks back prevented :: being symmetrical with
0.0.0.0 in its IsIPv4()/IsIPv6() results.

As it stands today, we default to 0.0.0.0 for IPv4-only systems (thus
--disable-ipv6 "works" as people are constantly pointing out). Default
to :: for IPv6-enabled systems, and leaving the caller code in full
responsibility of using SetPv4() when needed (like this bug) for
split-stack.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.15
   Beta testers wanted for 3.2.0.11