Hi Tim,

On Wed, Jul 18, 2018 at 01:48:01PM +0200, Tim Düsterhus wrote:
> This would solve the issue for my use case and should not break anything
> (a few UNKNOWNs will become TCP6 then).


> I can rework the patch, if technical changes look good to you. There's a
> ton of memcpy in there to not destroy the data structures needed for the
> actual communication: make_proxy_line() now always operates on a copy of
> `struct connection remote`.

I don't see why a copy could be needed, given that you should never have
to modify anything in the source. Or maybe it's because it was more
convenient to store the remapped addresses ? In this case I think that
it's still cleaner to have two local struct sockaddr_storage that you
can use for the operations. Another option, which I *thought* we had but
am probably wrong on this, was to first check the source+destination
address classes before deciding what to emit. (or maybe it's done in the
PPv2 format) :

     if (src_is_v4 && dst_is_v4)
          /* emit_v4(src, dst) */
     else if (src_is_v6 && dst_is_v6)
          /* emit_v6(src, dst) */
     else if (src_is_v6 && dst_is_v4)
          /* emit_v6(src, 4to6(dst)) */
     else if (src_is_v4 && dst_is_v6)
          /* emit_v6(4to6(src), dst) */
          /* emit_unknown()*/

So in practice you always need only one temporary sockaddr_storage for
a conversion.

I'm personally fine with something roughly like this. Lukas, I'm interested
in your opinion on this one, as I *think* it addresses the issues without
introducing new ones. We could even think about backporting this.

> I could not find a better solution and already thought hard about the
> current version.

I have some vague memories about this area a long time ago, and believe
me, when I have memories of some code parts it's likely that I got a
headache on it :-)


Reply via email to