Re: [PATCH] xt_TCPOPTSTRIP 20071006 (kernel)

[Patrick McHardy <kaber@trash.net>]

Jan Engelhardt wrote:
> Add xt_TCPOPTSTRIP, a module to replace TCP options by NOPs.

A few minor comments:

> +#define tcpoptstrip_set_bit(bmap, idx) \
> +	(bmap[(idx) >> 5] |= 1UL << (idx & 31))
> +#define tcpoptstrip_test_bit(bmap, idx) \
> +	(((1UL << (idx & 31)) & bmap[(idx) >> 5]) != 0)


Why UL? You're using 32 bit variables for the bitmap ..


> +struct xt_tcpoptstrip_info {
> +	u_int32_t strip_bmap[8];
> +};
> +


> +config NETFILTER_XT_TARGET_TCPOPTSTRIP
> +	tristate '"TCPOPTSTRIP" target support'
> +	depends on NETFILTER_XTABLES


EXPERIMENTAL please

> +	---help---
> +	  This option adds a "TCPOPTSTRIP" target, which allows you to strip
> +	  TCP options from TCP packets.


> +static unsigned int
> +xt_tcpoptstrip_target6(struct sk_buff **pskb, const struct net_device *in,
> +                       const struct net_device *out, unsigned int hooknum,
> +                       const struct xt_target *target, const void *targinfo)
> +{
> +	struct ipv6hdr *ipv6h = ipv6_hdr(*pskb);
> +	unsigned int tcphoff;
> +	u_int8_t nexthdr;
> +
> +	nexthdr = ipv6h->nexthdr;
> +	tcphoff = ipv6_skip_exthdr(*pskb, sizeof(*ipv6h), &nexthdr);


This needs #ifdef CONFIG_IPV6

A Signed-off-by: line by Sven would also be appreciated - I guess
you can just include the original one since you modified his
patch.