Hi,

On 08/12/2018 18:03, Gert Doering wrote:
> Hi,
> 
> On Sat, Dec 08, 2018 at 09:47:37AM +1000, Antonio Quartulli wrote:
>>> +               /* we only print port numbers for v4mapped v6 as of
>>> +                * today, because "v6addr:port" is too ambiguous
>>> +                */
>>
>> The comments above are indented with tab+spaces, while the code below is
>> just all spaces. Please use the all spaces everywhere.
> 
> Gah.  That was the intention, but the comment was added later on and
> using the wrong editor :-)
> 
>>> +                    if (maddr.type & MR_WITH_PORT)
>>> +                    {
>>> +                        buf_printf(&out, ":%d", ntohs(maddr.v6.port));
>>> +                    }
>>
>> I don't understand how is this solving the ambiguity? 
> 
> These are *v4* addresses, just masquerading as v6 in the socket structure.
> 
> So 1.2.3.4:567 is never ambiguous.
> 
> For "true v6 addresses" (the other branch) we keep on not printing the
> port number.
> 
>> Or you are just
>> saying: "we can't do much, let's just print the port anyway"? My
>> suggestion would be to surround the address with [], so basically change
>> the printf format above from %s to [%s]. Does it make sense?
> 
> This is something I want to discuss.  Changing mroute_print_addr_ex()
> for "true v6 addresses" and printing the port number in a new format
> will affect status file printing, management interface, etc. - so it
> needs to be well considered.
> 
> *This* patch just fixes the discrepancy that v4 addresses are printed
> "with port" if doing "proto udp4", and "without port" if a v4 connect
> comes in on a "proto udp / v6only=no" socket.  (We do not need to discuss
> how many problems the v4mapped v6 address format brings with it...)
> 

Thanks for the clarification.

The patch looks good and does what it says on the lid.
Tested by checking the status output (with and without the patch) and
everything looked good.

Now it only needs to have the tabs fixed on the comment lines, but other
than that:

Acked-by: Antonio Quartulli <anto...@openvpn.net>


Regards,

-- 
Antonio Quartulli

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to