On 23/04/2014 04:17, Arne Schwabe wrote:
Am 21.04.14 21:26, schrieb James Yonan:
On 21/04/2014 05:27, Arne Schwabe wrote:
On 21.04.2014 09:10, James Yonan wrote:
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.
I haven't look at the code but if remember correctly, this method does
not get passed the actual flags but the flags should *potientially* be
checked.
Yes, the point is to aggregate the potential flags of interest to
process_ip_header under a single mask, i.e. PIP_OPT_MASK.
So instead of having "if (flags & (PIPV4_PASSTOS|PIP_MSSFIX|...|...))"
you just have "if (flags & PIP_OPT_MASK)".
The main issue for us is that we have an internal patch for one of our
services that adds an additional flag, and the previous usage is a
merge conflict attractor because all the flags of interest are
explicitly given on a single line.
I only wanted to point out my patch also exits. I looked at the code again.
I think to really get rid of the #ifdef here we also need to
unconditionally enable the other &= ~ statements:
Change
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
flags &= ~PIPV4_PASSTOS;
#endif
to
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
#endif
flags &= ~PIPV4_PASSTOS;
Otherwise if the function is called with PIPV4_PASSTOS but not compiled
with PASSTOS_CAPABILITY the
flags & PIP_OPT_MASK will be always true.
Arne
Sure, that makes sense.
James
