On 01/08/11 20:52, Tsantilas Christos wrote:
On 07/29/2011 06:50 PM, Alex Rousskov wrote:
On 07/25/2011 08:59 AM, Tsantilas Christos wrote:
I am sending a version 4 of the patch which is the same with Amos patch
but is a little smaller. IT is easier to see the changes.


bool
Ip::Address::IsIPv4() const
{
- return IsAnyAddr() || IsNoAddr() ||
IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
+ return IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
}

bool
Ip::Address::IsIPv6() const
{
- return IsAnyAddr() || IsNoAddr() ||
!IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
+ return !IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
}

Can we rewrite IsIPv6() as "return !IsIPv4()", to clarify the intent if
that is indeed the intent?

Also, the documentation for these two methods seems to imply a different
relationship, at least in some corner cases:

/** Test whether content can be used as an IPv4 address
\retval true if content was received as an IPv4 address
\retval true if content was received as an IPv4-Mapped address
\retval false if content was received as a non-mapped IPv6 native
address.
*/
bool IsIPv4() const;

/** Test whether content can be used as an IPv6 address.
\retval true if --enable-ipv6 has been compiled.
\retval false if --disable-ipv6 has been compiled.
\retval false if --with-ipv6-split-stack has been compiled AND
content is I
Pv4-mapped.
*/
bool IsIPv6() const;

As far as I can tell, the above definitions make it possible for both
IsIPv4() and IsIPv6() to return true at the same time in some cases, but
the implementation does not support that. Thus, the docs or the
implementation is wrong.

We should rename the IsIPv6() method to IsIpv6Only(), to avoid
confusion. And then maybe implement an new IsIPv6 method, which for now
will return always true (my sense is that everything can be an ipv6
address).


"everything is IPv6 address" is the hybrid model. My work from 2007 was designed around that. The fine details distinctions between the variations of TCP stack appeared and bit in a bad way. Each stack places different limitations on what can be converted and used where. In split-stack and basic dual-stack the v4-mapped addresses are invalid and cant be called ether v4 or v6. Just an intermediate state that can be stored in v6 structures and used/converted to v4-only at need.

The IsIPvN() calls both have the implicit "only" needed for split-stack use at present. This patch is enforcing and polishing that implict detail to solid, not changing the semantics.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.14
  Beta testers wanted for 3.2.0.10

Reply via email to