On 04/03/2016 02:49, Arne Schwabe wrote:
Am 04.03.16 um 08:29 schrieb James Yonan:
On 03/03/2016 16:48, Arne Schwabe wrote:
Am 03.03.16 um 09:18 schrieb James Yonan:
Define PIP_OPT_MASK to represent all flags of interest to
process_ip_header, so that it can have a fast exit path
if no flags are set.
Basically what this patch does is to change the condition to
if (flags)
and if for example PASSTOS_CAPABILITY is not 1, the following path will
always be taken:
process_ip_header (c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
&c->c2.buf);
flags mean that possible passtos, mssfix and client_nat should be
applied here.
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
flags &= ~PIPV4_PASSTOS;
#endif
is not compiled in. So flags is at least PIPV4_PASSTOS
So if (flags & 0xffff) is still true.
So NACK from me butthe code is very confusing...
Arne
I think what makes this patch confusing is that it's really a patch
that facilitates another patch that we've used in the past at OpenVPN
Tech. for some custom NAT algs. This patch reduces the footprint of
the second patch, making it easier to maintain.
Yes. I think I get the intention of the patch. But if this patch should
work in OpenVPN now we need to replace
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
flags &= ~PIPV4_PASSTOS;
#endif
with
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
#endif
flags &= ~PIPV4_PASSTOS;
to have the correct behaviour if PASSTOS_CAPABILITY is missing.
Actually, I'm fine to withdraw this patch for now -- it's probably more
trouble than it's worth.
James
