Re: [PATCH] netfilter: ipt_CLUSTERIP: dont flood with "no conntrack!"

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 13 janvier 2011 à 12:28 +0100, Patrick McHardy a écrit :
> On 13.01.2011 12:23, Pablo Neira Ayuso wrote:
> > Hi Eric,
> > 
> > On 13/01/11 12:13, Eric Dumazet wrote:
> >> ipt_CLUSTERIP users might hit this annoying printk, if they forgot an
> >> "iptables -I INPUT -m state --state INVALID -j DROP" before CLUSTERIP
> >> rule. We could use net_ratelimit() here, or not log the message at all.
> >> I chose to log it once per config.
> > 
> > I think that this should be converted to pr_debug() instead, there's
> > also another reference to "unknown protocol" that should be converted as
> > well.
> 
> I think the FIXME could also be removed, we *do* drop invalid
> packets in CLUSTERIP.

Ah yes indeed :)

Thanks !

[PATCH v3] netfilter: ipt_CLUSTERIP: dont flood with "no conntrack!"

ipt_CLUSTERIP users might hit this annoying printk, if they forgot an
"iptables -I INPUT -m state --state INVALID -j DROP" before CLUSTERIP
rule. We could use net_ratelimit() here, or not log the message at all.
I chose to log it once per config.

Pablo suggested to use same logic for the "unknown protocol" message
Patrick asked to remove an obsolete comment.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
CC: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 1e26a48..b5cf3e4 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -47,6 +47,8 @@ struct clusterip_config {
 	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
 	struct net_device *dev;			/* device */
 	u_int16_t num_total_nodes;		/* total number of nodes */
+	bool warned_no_conntrack;
+	bool warned_unknown_protocol;
 	unsigned long local_nodes;		/* node number array */
 
 #ifdef CONFIG_PROC_FS
@@ -228,7 +230,7 @@ clusterip_del_node(struct clusterip_config *c, u_int16_t nodenum)
 
 static inline u_int32_t
 clusterip_hashfn(const struct sk_buff *skb,
-		 const struct clusterip_config *config)
+		 struct clusterip_config *config)
 {
 	const struct iphdr *iph = ip_hdr(skb);
 	unsigned long hashval;
@@ -236,7 +238,7 @@ clusterip_hashfn(const struct sk_buff *skb,
 	int poff;
 
 	poff = proto_ports_offset(iph->protocol);
-	if (poff >= 0) {
+	if (likely(poff >= 0)) {
 		const u_int16_t *ports;
 		u16 _ports[2];
 
@@ -246,8 +248,10 @@ clusterip_hashfn(const struct sk_buff *skb,
 			dport = ports[1];
 		}
 	} else {
-		if (net_ratelimit())
+		if (unlikely(!config->warned_unknown_protocol)) {
+			config->warned_unknown_protocol = true;
 			pr_info("unknown protocol %u\n", iph->protocol);
+		}
 	}
 
 	switch (config->hash_mode) {
@@ -301,10 +305,10 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
 
 	ct = nf_ct_get(skb, &ctinfo);
 	if (ct == NULL) {
-		pr_info("no conntrack!\n");
-			/* FIXME: need to drop invalid ones, since replies
-			 * to outgoing connections of other nodes will be
-			 * marked as INVALID */
+		if (unlikely(!cipinfo->config->warned_no_conntrack)) {
+			cipinfo->config->warned_no_conntrack = true;
+			pr_info("no conntrack!\n");
+		}
 		return NF_DROP;
 	}
 


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html