On 07/08/2012 07:58 AM, Jan Engelhardt wrote:
On Saturday 2012-07-07 23:20, Josh Hunt wrote:
Moves TCP header manipulation into a separate function to allow code-sharing for
IPv6 support.
Hi Josh,
in your patch, you seem to oversee that the tarpit_generic block that
you are moving into a new function had "return;"s to exit from
the tarpit_tcp function. With your change,
+ niph->tot_len = htons(nskb->len);
+ tcph->urg_ptr = 0;
+ /* Reset flags */
+ ((u_int8_t *)tcph)[13] = 0;
+
+ tarpit_generic(oth, tcph, payload, mode);
/* Adjust TCP checksum */
tcph->check = 0;
The checksum code (and what follows) it will be executed in all cases,
even though TCP RST are supposed not to get any reply per the comment.
tarpit_generic would have to return a bool value propagating this
exit style. In other words,
static bool tarpit_generic
{
if (mode == XTTARPIT_TARPIT)
/* No replies for RST, FIN , ... */
if (oth->rst ...)
return false;
}
...
return true;
}
static void tarpit_tcp
{
...
if (!tarpit_generic())
return;
tcph->check = 0;
...
}
Doh! Yep totally overlooked that. Thanks for catching. Will update and
resend.
It would also be appreciated if you could move each case
(XTTARPIT_{TARPIT,HONEYPOT,RESET}) into their own functions in three
patches added to the front of the set.
(Reduces indent and makes the diffs simpler.)
Sure. I'll do this as well.
Thanks for the review.
Josh
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
