References: <1456993146-63968-1-git-send-email-james@...515...>
	<1456993146-63968-2-git-send-email-james@...515...>
	<56D8CD3E.2070403@...1227...> <56D93976.1090906@...515...>
From: Arne Schwabe <arne@...1227...>
Message-ID: <56D95A46.9060802@...1227...>
Date: Fri, 4 Mar 2016 10:49:58 +0100
MIME-Version: 1.0
In-Reply-To: <56D93976.1090906@...515...>
Subject: Re: [Openvpn-devel] [PATCH 02/10] Added PIP_OPT_MASK for
 process_ip_header fast exit path.
List-Id: <openvpn-devel.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/openvpn-devel>, 
	<mailto:openvpn-devel-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=openvpn-devel>
List-Post: <mailto:openvpn-devel@lists.sourceforge.net>
List-Help: <mailto:openvpn-devel-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/openvpn-devel>, 
	<mailto:openvpn-devel-request@lists.sourceforge.net?subject=subscribe>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
To: James Yonan <james@...515...>, openvpn-devel@lists.sourceforge.net



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.

Arne