Hi Jarno,

On Sun, Jun 04, 2017 at 07:12:38PM +0300, Jarno Huuskonen wrote:
> Hi Willy,
> 
> On Sat, May 27, Willy Tarreau wrote:
> > On Sun, May 21, 2017 at 07:49:00PM +0300, Jarno Huuskonen wrote:
> > > I noticed that ipv6 version of ipmask() converter is missing ?
> > > 
> > > I'm attaching an example implementation for ipv6mask (incomplete:
> > > missing at least documentation / lua) for comments.
> > > 
> > > Maybe instead of just working with one mask ipv6mask(converter) could
> > > take two arguments: first ipv6mask, second(optional) ipv4 mask applied to
> > > ipv4mapped ipv6 address(::ffff:1.2.3.4).
> > 
> > That could indeed be a good idea. Thinking a bit further, shouldn't we
> > extend the current ipmask to support an optional ipv6 mask as a second
> > argument (ie exactly what you did but with ipv6 optional instead of
> > ipv4) ? This would allow it to be used on any address. It would end
> > up like this :
> >   - ipv4 in => ipv4 out, using ipv4 mask
> >   - ipv4-in-ipv6 in => ipv4-in-ipv6 out, using ipv4 mask
> >   - ipv6 in => ipv6 out, using ipv6 mask
> 
> So ipmask would internally work on ipv6 addresses and ipv4
> addresses are converted to/from mapped (::ffff:1.2.3.4) addresses ?

No, not necessarily, we know the type of the input sample so we can apply
the mask we want and emit the same type on output. I was only saying that
the ipv4 mask should be used when you face ipv4-in-ipv6 addresses.

> > What do you think ?
> 
> ipmask() with optional ipv6mask is fine with me, I guess it won't break
> existing configs because ipv6 mask is optional. For ipv4 only it probably
> adds a little overhead when converting to/from mapped ::ffff:1.2.3.4 
> addresses.

It would cause other trouble such as conversion to strings which will be
mangled by this "::ffff:" prefix. That's why it's better to maintain the
same type.

> @@ -2736,7 +2747,7 @@ static struct sample_conv_kw_list sample_conv_kws = 
> {ILH, {
>       { "upper",  sample_conv_str2upper, 0,            NULL, SMP_T_STR,  
> SMP_T_STR  },
>       { "lower",  sample_conv_str2lower, 0,            NULL, SMP_T_STR,  
> SMP_T_STR  },
>       { "hex",    sample_conv_bin2hex,   0,            NULL, SMP_T_BIN,  
> SMP_T_STR  },
> -     { "ipmask", sample_conv_ipmask,    ARG1(1,MSK4), NULL, SMP_T_IPV4, 
> SMP_T_IPV4 },
> +     { "ipmask", sample_conv_ipmask,    ARG2(1,MSK4,MSK6), NULL, SMP_T_IPV6, 
> SMP_T_IPV6 },

You need to use SMP_T_ADDR instead of SMP_T_IPV6 in order to adapt to any
address family type and not enforce it. Maybe this is what led you to think
that you needed to convert ipv4 to ipv6.

Cheers,
Willy

Reply via email to