Re: [PATCH 1/3] xtables: tarpit: Make tarpit code generic

[Jan Engelhardt <jengelh@xxxxxxx>]

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;
	...
}

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.)
--
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