Re: [PATCH] IPv6 transparent proxy support for Squid

Hi,

Thanks for your reply and your comments.

I've joined a patch which include your remarks.

Alexis

On Thu, Jun 20, 2013 at 4:08 PM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:

> On 20/06/2013 9:20 p.m., Alexis Robert wrote:
>
>> Hi,
>>
>> As NAT support has been included for IPv6 in Linux 3.7 (along with
>> REDIRECT/DNAT rules), as well as IP6T_SO_ORIGINAL_DST in Linux 3.8, I wrote
>> a little patch to add support for transparent proxies over IPv6.
>>
>> It's my first patch for Squid, I hope I've done it correctly :) The patch
>> is based on the bzr repository.
>>
>>
> It looks better than most first submissions. Thank you.
>
>
> The only thing I see which can cause trouble is a bug with
>> linux/netfilter_ipv6/ip6_**tables.h on C++ compilers, the bug report and
>> the patch to fix it are here : https://lkml.org/lkml/2012/9/**30/146<https://lkml.org/lkml/2012/9/30/146>.
>> It's only used for the constant IP6T_SO_ORIGINAL_DST. Maybe there is a
>> better (and clean) way to do so it doesn't force users to update their
>> header file, but I've haven't found it yet.
>>
>> Have a nice day,
>>
>> Alexis
>>
>
> Audit results:
>
> When we are manipulating a sockaddr which may be either sockaddr_in
> /sockaddr_in6 type it is preferrable to use the sockaddr_storage or
> addrinfo types.
>
> * please just change the type of "lookup" to sockaddr_storage. That and
> passing AF_UNSPEC as the second parameter to local.getSockAddr() will take
> care of the IPv4/IPv6 differences without needing most of the new
> if-statements.
>
> * in the debugs() statement removing the "SO_" so that the text just says
> "getsockopt(ORIGINAL_DST)" should be sufficient to remove the remaining
> if-statement. It will be clear from the printed socket IP:port details
> which option is in use.
>
> * please remove the new empty line before the return statement.
>
>
> Amos
>