Re: [patch] netfilter: implement TCPMSS target for IPv6

[Patrick McHardy <kaber@trash.net>]

Jan Engelhardt wrote:
> On Jan 15 2007 19:42, Patrick McHardy wrote:
> 
>>+#endif /* _XT_TCPMSS_H*/
> 
>                         ^
> nitpick: space
> 
> 
>>+	  iptables -A FORWARD -p tcp --tcp-flags SYN,RST SYN \
>>+	                 -j TCPMSS --clamp-mss-to-pmtu
> 
> 
> This should probably be --tcp-flags SYN,RST,FIN SYN or just --syn.

Both fixed, thanks.

> 
> 
>>+MODULE_LICENSE("GPL");
>>+MODULE_AUTHOR("Marc Boucher <marc@mbsi.ca>");
>>+MODULE_DESCRIPTION("x_tables TCP MSS modification module");
>>+MODULE_ALIAS("ipt_TCPMSS");
>>+MODULE_ALIAS("ip6t_TCPMSS");
> 
> 
> One thing that comes to mind: what is the exact name of it? xtables?
> x_tables? In ipt_*, one could also see "IP tables" and so on. The
> actual userspace program is called 'iptables', so I'd go for
> 'xtables'. Though, it's just a userspace tool. Someone could come
> along and make a SuperTables that still works with the existing
> netfilter code in the kernel, making it 'netfilter TCP MSS
> modification module' rather than 'x_tables...'. Well, before my
> thoughts crumble, let me know something :)

Most modules use x_tables in their description. I don't care
at all.

>>+static int
>>+xt_tcpmss_checkentry4(const char *tablename,
>>+		      const void *entry,
>>+		      const struct xt_target *target,
>>+		      void *targinfo,
>>+		      unsigned int hook_mask)
>>+{
>>+	const struct xt_tcpmss_info *info = targinfo;
>>+	const struct ipt_entry *e = entry;
>>+
>>+	if (info->mss == XT_TCPMSS_CLAMP_PMTU &&
>>+	    (hook_mask & ~((1 << NF_IP_FORWARD) |
>>+			   (1 << NF_IP_LOCAL_OUT) |
>>+			   (1 << NF_IP_POST_ROUTING))) != 0) {
>>+		printk("xt_TCPMSS: path-MTU clamping only supported in "
>>+		       "FORWARD, OUTPUT and POSTROUTING hooks\n");
>>+		return 0;
>>+	}
>>+	if (IPT_MATCH_ITERATE(e, find_syn_match))
>>+		return 1;
>>+	printk("xt_TCPMSS: Only works on TCP SYN packets\n");
>>+	return 0;
>>+}
> 
> 
> The hook_mask check could be done with struct xt_target->hooks, could
> not it? (see this:)

No, the hooks only matter for pmtu clamping.