[PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[Duncan Palmer]

Hi,

Whilst working on a netfilter module, I noticed a
comment in ipt_REJECT.c saying 'FIXME: Use sysctl
number. --RR' before a call to xrlim_allow(). This is
fixed in the attached patch (against 2.6.11-rc2-mm1,
but applies cleanly to rc3-mm1 as well).

Dunk

diff -ur linux-2.6.11-rc2-mm1.orig/net/ipv4/icmp.c
linux-2.6.11-rc2-mm1/net/ipv4/icmp.c
--- linux-2.6.11-rc2-mm1.orig/net/ipv4/icmp.c
2005-01-28 13:01:17.000000000 +1100
+++ linux-2.6.11-rc2-mm1/net/ipv4/icmp.c	2005-02-08
15:07:11.000000000 +1100
@@ -208,6 +208,8 @@
 int sysctl_icmp_ratelimit = 1 * HZ;
 int sysctl_icmp_ratemask = 0x1818;
 
+EXPORT_SYMBOL(sysctl_icmp_ratelimit);
+
 /*
  *	ICMP control array. This specifies what to do with
each ICMP.
  */
diff -ur
linux-2.6.11-rc2-mm1.orig/net/ipv4/netfilter/ipt_REJECT.c
linux-2.6.11-rc2-mm1/net/ipv4/netfilter/ipt_REJECT.c
---
linux-2.6.11-rc2-mm1.orig/net/ipv4/netfilter/ipt_REJECT.c
2005-01-28 13:01:18.000000000 +1100
+++
linux-2.6.11-rc2-mm1/net/ipv4/netfilter/ipt_REJECT.c
2005-02-08 15:02:42.000000000 +1100
@@ -234,8 +234,7 @@
 	if (!rt)
 		return;
 
-	/* FIXME: Use sysctl number. --RR */
-	if (!xrlim_allow(&rt->u.dst, 1*HZ))
+	if (!xrlim_allow(&rt->u.dst, sysctl_icmp_ratelimit))
 		return;
 
 	iph = skb_in->nh.iph;
diff -ur
linux-2.6.11-rc2-mm1.orig/net/ipv4/sysctl_net_ipv4.c
linux-2.6.11-rc2-mm1/net/ipv4/sysctl_net_ipv4.c
---
linux-2.6.11-rc2-mm1.orig/net/ipv4/sysctl_net_ipv4.c
2004-12-25 08:35:23.000000000 +1100
+++ linux-2.6.11-rc2-mm1/net/ipv4/sysctl_net_ipv4.c
2005-02-08 15:07:54.000000000 +1100
@@ -15,6 +15,7 @@
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/tcp.h>
+#include <net/icmp.h>
 
 /* From af_inet.c */
 extern int sysctl_ip_nonlocal_bind;
@@ -34,7 +35,6 @@
 extern int sysctl_ip_dynaddr;
 
 /* From icmp.c */
-extern int sysctl_icmp_ratelimit;
 extern int sysctl_icmp_ratemask;
 
 /* From igmp.c */
diff -ur linux-2.6.11-rc2-mm1.orig/include/net/icmp.h
linux-2.6.11-rc2-mm1/include/net/icmp.h
--- linux-2.6.11-rc2-mm1.orig/include/net/icmp.h
2005-01-28 13:01:11.000000000 +1100
+++ linux-2.6.11-rc2-mm1/include/net/icmp.h	2005-02-08
15:07:04.000000000 +1100
@@ -38,6 +38,8 @@
 #define ICMP_INC_STATS_BH(field)
SNMP_INC_STATS_BH(icmp_statistics, field)
 #define ICMP_INC_STATS_USER(field) 
SNMP_INC_STATS_USER(icmp_statistics, field)
 
+extern int sysctl_icmp_ratelimit;
+
 extern void	icmp_send(struct sk_buff *skb_in,  int
type, int code, u32 info);
 extern int	icmp_rcv(struct sk_buff *skb);
 extern int	icmp_ioctl(struct sock *sk, int cmd,
unsigned long arg);


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: [PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[Duncan Palmer]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

--- Duncan Palmer <dunk_palmer@yahoo.com> wrote:

> fixed in the attached patch (against 2.6.11-rc2-mm1,
> but applies cleanly to rc3-mm1 as well).

Bugger - yahoo has mangled the patch - sorry. I've
attached it instead...

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sysctl.patch --]
[-- Type: text/x-patch; name="sysctl.patch", Size: 2266 bytes --]

diff -ur linux-2.6.11-rc2-mm1.orig/net/ipv4/icmp.c linux-2.6.11-rc2-mm1/net/ipv4/icmp.c
--- linux-2.6.11-rc2-mm1.orig/net/ipv4/icmp.c	2005-01-28 13:01:17.000000000 +1100
+++ linux-2.6.11-rc2-mm1/net/ipv4/icmp.c	2005-02-08 15:07:11.000000000 +1100
@@ -208,6 +208,8 @@
 int sysctl_icmp_ratelimit = 1 * HZ;
 int sysctl_icmp_ratemask = 0x1818;
 
+EXPORT_SYMBOL(sysctl_icmp_ratelimit);
+
 /*
  *	ICMP control array. This specifies what to do with each ICMP.
  */
diff -ur linux-2.6.11-rc2-mm1.orig/net/ipv4/netfilter/ipt_REJECT.c linux-2.6.11-rc2-mm1/net/ipv4/netfilter/ipt_REJECT.c
--- linux-2.6.11-rc2-mm1.orig/net/ipv4/netfilter/ipt_REJECT.c	2005-01-28 13:01:18.000000000 +1100
+++ linux-2.6.11-rc2-mm1/net/ipv4/netfilter/ipt_REJECT.c	2005-02-08 15:02:42.000000000 +1100
@@ -234,8 +234,7 @@
 	if (!rt)
 		return;
 
-	/* FIXME: Use sysctl number. --RR */
-	if (!xrlim_allow(&rt->u.dst, 1*HZ))
+	if (!xrlim_allow(&rt->u.dst, sysctl_icmp_ratelimit))
 		return;
 
 	iph = skb_in->nh.iph;
diff -ur linux-2.6.11-rc2-mm1.orig/net/ipv4/sysctl_net_ipv4.c linux-2.6.11-rc2-mm1/net/ipv4/sysctl_net_ipv4.c
--- linux-2.6.11-rc2-mm1.orig/net/ipv4/sysctl_net_ipv4.c	2004-12-25 08:35:23.000000000 +1100
+++ linux-2.6.11-rc2-mm1/net/ipv4/sysctl_net_ipv4.c	2005-02-08 15:07:54.000000000 +1100
@@ -15,6 +15,7 @@
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/tcp.h>
+#include <net/icmp.h>
 
 /* From af_inet.c */
 extern int sysctl_ip_nonlocal_bind;
@@ -34,7 +35,6 @@
 extern int sysctl_ip_dynaddr;
 
 /* From icmp.c */
-extern int sysctl_icmp_ratelimit;
 extern int sysctl_icmp_ratemask;
 
 /* From igmp.c */
diff -ur linux-2.6.11-rc2-mm1.orig/include/net/icmp.h linux-2.6.11-rc2-mm1/include/net/icmp.h
--- linux-2.6.11-rc2-mm1.orig/include/net/icmp.h	2005-01-28 13:01:11.000000000 +1100
+++ linux-2.6.11-rc2-mm1/include/net/icmp.h	2005-02-08 15:07:04.000000000 +1100
@@ -38,6 +38,8 @@
 #define ICMP_INC_STATS_BH(field)	SNMP_INC_STATS_BH(icmp_statistics, field)
 #define ICMP_INC_STATS_USER(field) 	SNMP_INC_STATS_USER(icmp_statistics, field)
 
+extern int sysctl_icmp_ratelimit;
+
 extern void	icmp_send(struct sk_buff *skb_in,  int type, int code, u32 info);
 extern int	icmp_rcv(struct sk_buff *skb);
 extern int	icmp_ioctl(struct sock *sk, int cmd, unsigned long arg);

Re: [PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[Patrick McHardy]

Duncan Palmer wrote:

>Hi,
>
>Whilst working on a netfilter module, I noticed a
>comment in ipt_REJECT.c saying 'FIXME: Use sysctl
>number. --RR' before a call to xrlim_allow(). This is
>fixed in the attached patch (against 2.6.11-rc2-mm1,
>but applies cleanly to rc3-mm1 as well).
>
I was never convinced of using xrlim_allow in ipt_REJECT. We
already have the limit match, which allows to set limits in
the ruleset. The current of of xrlim_allow is actually broken,
the max number of tokens in the dst_entry is limited by the timeout
value, so both icmp.c and ipt_REJECT.c are limited by the smaller
of both timeouts when ICMPs are sent by both. ipt_REJECT also
doesn't care about icmp_ratemask and uses up token for types
that shouldn't be limited. So we can either remove it entirely
or use icmpv4_xrlim_allow. If there are no objections I would
prefer to remove it.

Regards
Patrick

Re: [PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[Duncan Palmer]

--- Patrick McHardy <kaber@trash.net> wrote:

> I was never convinced of using xrlim_allow in
> ipt_REJECT. We
> already have the limit match, which allows to set
> limits in
> the ruleset. 

Is there anywhere that sends icmp pkts apart from
icmp.c and ipt_REJECT.c? (a quick look suggests not).
If not, then one problem I can see with removing
xrlim_allow() from ipt_REJECT is that its behaviour
will become inconsistent with that of icmp.c, in that
it doesn't use sysctl for setting rlimit (not that it
ever has...)

> that shouldn't be limited. So we can either remove
> it entirely
> or use icmpv4_xrlim_allow. If there are no
> objections I would prefer to remove it.

After reading the relevant bits of the RFC and a bit
more code, I agree that xrlim_allow() is indeed
buggered...

I'm far from being an expert on linux's networking
internals, but it seems to me that many aspects of the
operation of network stacks are configurable using
sysctl variables. Not calling icmpv4_xrlim_allow()
will make the icmp ratelimit parameter a bit of an odd
one out as far as ipv4 is concerned, as I think there
are other ipv4 sysctl parameters who's functionality
could similarly be replaced by iptables...

I'll be happy do do up a patch on whatever is decided
upon anyway...

Dunk



		
__________________________________ 
Do you Yahoo!? 
The all-new My Yahoo! - Get yours free! 
http://my.yahoo.com 
 

Re: [PATCH] Get icmp ratelimit from sysctl in ipt_REJECT.c

[Patrick McHardy]

Duncan Palmer wrote:

>Is there anywhere that sends icmp pkts apart from
>icmp.c and ipt_REJECT.c? (a quick look suggests not).
>If not, then one problem I can see with removing
>xrlim_allow() from ipt_REJECT is that its behaviour
>will become inconsistent with that of icmp.c, in that
>it doesn't use sysctl for setting rlimit (not that it
>ever has...)
>
It doesn't has to be consistent with icmp.c, but it should be
consistent with the remainder of iptables, this means to do as
the ruleset says.

>After reading the relevant bits of the RFC and a bit
>more code, I agree that xrlim_allow() is indeed
>buggered...
>
>I'm far from being an expert on linux's networking
>internals, but it seems to me that many aspects of the
>operation of network stacks are configurable using
>sysctl variables. Not calling icmpv4_xrlim_allow()
>will make the icmp ratelimit parameter a bit of an odd
>one out as far as ipv4 is concerned, as I think there
>are other ipv4 sysctl parameters who's functionality
>could similarly be replaced by iptables...
>
ipt_REJECT is different from icmp.c, it doesn't send ICMP messages
in response to error conditions but because the admin said so in
his ruleset. If he wants to limit it he can use the limit match.

>I'll be happy do do up a patch on whatever is decided
>upon anyway...
>
I already removed it in my tree, but haven't committed it yet.

Regards
Patrick