Consolidate IP fragment management

PATCH Applied Pavel Emelyanov 2 participants 20 messages Oct 12, 2007 – Oct 15, 2007

Subsystems (4)

Networking Network drivers netfilter IPv4/IPv6

lore

Share & tools

lore

Pavel Emelyanov

author

2007-10-12 12:57 UTC #0 lore raw

cover Consolidate IP fragment management

Patrick recently pointed out, that there are three places that perform IP fragments management. In ipv4, ipv6 and in ip6 conntracks. Looks like these places can be a bit consolidated.

The proposal is to create a common structure inet_frag_queue to put common fields like list heads, refcounts etc in, and include it into the specific fragment queues. Then such objects like hash tables, lists, locks etc are moved to common place (struct inet_frags). At the end common code is moved to the net/ipv4/inet_fragment.c.

The inet_ prefix in file names, data structures and functions, and the code place (net/ipv4) was proposed by Alexey, but the exact names were selectd by me, so maybe there can be a better ones.

Signed-off-by Pavel Emelyanov <[email protected]>

Pavel Emelyanov

author

2007-10-12 13:02 UTC #1 lore raw

PATCH 1/9 Move common fields from frag_queues in one place

Introduce the struct inet_frag_queue in include/net/inet_frag.h file and place there all the common fields from three structs:

* struct ipq in ipv4/ip_fragment.c
* struct nf_ct_frag6_queue in nf_conntrack_reasm.c
* struct frag_queue in ipv6/reassembly.c

After this, replace these fields on appropriate structures with this structure instance and fix the users to use correct names i.e. hunks like

- atomic_dec(&fq->refcnt);
+ atomic_dec(&fq->q.refcnt);

(these occupy most of the patch)

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h new file +21 -0

new file mode 100644
index 0000000..74e9cb9
@@ -0,0 +1,21 @@
+#ifndef __NET_FRAG_H__
+#define __NET_FRAG_H__
+
+struct inet_frag_queue {
+	struct hlist_node	list;
+	struct list_head	lru_list;   /* lru list member */
+	spinlock_t		lock;
+	atomic_t		refcnt;
+	struct timer_list	timer;      /* when will this queue expire? */
+	struct sk_buff		*fragments; /* list of received fragments */
+	ktime_t			stamp;
+	int			len;        /* total length of orig datagram */
+	int			meat;
+	__u8			last_in;    /* first/last segment arrived? */
+
+#define COMPLETE		4
+#define FIRST_IN		2
+#define LAST_IN			1
+};
+
+#endif

net/ipv4/ip_fragment.c +80 ~~-91~~

412 lines not on this page · raw message

net/ipv6/netfilter/nf_conntrack_reasm.c +63 ~~-74~~

348 lines not on this page · raw message

net/ipv6/reassembly.c +68 ~~-79~~

363 lines not on this page · raw message

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:24 UTC #2 lore raw

Re: [PATCH 1/9] Move common fields from frag_queues in one place

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:00:06 +0400

Quoted 14 lines

Introduce the struct inet_frag_queue in include/net/inet_frag.h file and place there all the common fields from three structs:
* struct ipq in ipv4/ip_fragment.c
* struct nf_ct_frag6_queue in nf_conntrack_reasm.c
* struct frag_queue in ipv6/reassembly.c
After this, replace these fields on appropriate structures with this structure instance and fix the users to use correct names i.e. hunks like
(quoted diff) +1 -1
-    atomic_dec(&fq->refcnt);
+    atomic_dec(&fq->q.refcnt);

Quoted 3 lines

(these occupy most of the patch)
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:08 UTC #3 lore raw

PATCH 2/9 Collect frag queues management objects together

In reply to Pavel Emelyanov

There are some objects that are common in all the places which are used to keep track of frag queues, they are:

* hash table
* LRU list
* rw lock
* rnd number for hash function
* the number of queues
* the amount of memory occupied by queues
* secret timer

Move all this stuff into one structure (struct inet_frags) to make it possible use them uniformly in the future. Like with the previous patch this mostly consists of hunks like

- write_lock(&ipfrag_lock);
+ write_lock(&ip4_frags.lock);

To address the issue with exporting the number of queues and the amount of memory occupied by queues outside the .c file they are declared in, I introduce a couple of helpers.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +15 -0

index 74e9cb9..d51f238 100644
@@ -18,4 +18,19 @@ struct inet_frag_queue {
 #define LAST_IN			1
 };
 
+#define INETFRAGS_HASHSZ		64
+
+struct inet_frags {
+	struct list_head	lru_list;
+	struct hlist_head	hash[INETFRAGS_HASHSZ];
+	rwlock_t		lock;
+	u32			rnd;
+	int			nqueues;
+	atomic_t		mem;
+	struct timer_list	secret_timer;
+};
+
+void inet_frags_init(struct inet_frags *);
+void inet_frags_fini(struct inet_frags *);
+
 #endif

include/net/ip.h +2 -2

index 3af3ed9..a18dcec 100644
@@ -333,8 +333,8 @@ enum ip_defrag_users
 };
 
 struct sk_buff *ip_defrag(struct sk_buff *skb, u32 user);
-extern int ip_frag_nqueues;
-extern atomic_t ip_frag_mem;
+int ip_frag_mem(void);
+int ip_frag_nqueues(void);
 
 /*
  *	Functions provided by ip_forward.c

include/net/ipv6.h +2 -2

index 31b3f1b..77cdab3 100644
@@ -252,8 +252,8 @@ struct ipv6_txoptions *ipv6_fixup_options(struct ipv6_txoptions *opt_space,
 
 extern int ipv6_opt_accepted(struct sock *sk, struct sk_buff *skb);
 
-extern int ip6_frag_nqueues;
-extern atomic_t ip6_frag_mem;
+int ip6_frag_nqueues(void);
+int ip6_frag_mem(void);
 
 #define IPV6_FRAG_TIMEOUT	(60*HZ)		/* 60 seconds */

net/ipv4/Makefile +2 -1

index a02c36d..93fe396 100644
@@ -10,7 +10,8 @@ obj-y     := route.o inetpeer.o protocol.o \
 	     tcp_minisocks.o tcp_cong.o \
 	     datagram.o raw.o udp.o udplite.o \
 	     arp.o icmp.o devinet.o af_inet.o  igmp.o \
-	     sysctl_net_ipv4.o fib_frontend.o fib_semantics.o
+	     sysctl_net_ipv4.o fib_frontend.o fib_semantics.o \
+	     inet_fragment.o
 
 obj-$(CONFIG_IP_FIB_HASH) += fib_hash.o
 obj-$(CONFIG_IP_FIB_TRIE) += fib_trie.o

net/ipv4/inet_fragment.c new file +44 -0

44 lines not on this page · raw message

net/ipv4/ip_fragment.c +53 ~~-56~~

266 lines not on this page · raw message

net/ipv4/proc.c +2 -2

index e5b05b0..fd16cb8 100644
@@ -70,8 +70,8 @@ static int sockstat_seq_show(struct seq_file *seq, void *v)
 	seq_printf(seq, "UDP: inuse %d\n", fold_prot_inuse(&udp_prot));
 	seq_printf(seq, "UDPLITE: inuse %d\n", fold_prot_inuse(&udplite_prot));
 	seq_printf(seq, "RAW: inuse %d\n", fold_prot_inuse(&raw_prot));
-	seq_printf(seq,  "FRAG: inuse %d memory %d\n", ip_frag_nqueues,
-		   atomic_read(&ip_frag_mem));
+	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
+			ip_frag_nqueues(), ip_frag_mem());
 	return 0;
 }

net/ipv6/netfilter/nf_conntrack_reasm.c +49 ~~-59~~

272 lines not on this page · raw message

net/ipv6/proc.c +1 -1

index db94501..be526ad 100644
@@ -54,7 +54,7 @@ static int sockstat6_seq_show(struct seq_file *seq, void *v)
 	seq_printf(seq, "RAW6: inuse %d\n",
 		       fold_prot_inuse(&rawv6_prot));
 	seq_printf(seq, "FRAG6: inuse %d memory %d\n",
-		       ip6_frag_nqueues, atomic_read(&ip6_frag_mem));
+		       ip6_frag_nqueues(), ip6_frag_mem());
 	return 0;
 }

net/ipv6/reassembly.c +54 ~~-56~~

269 lines not on this page · raw message

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:32 UTC #4 lore raw

Re: [PATCH 2/9] Collect frag queues management objects together

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:06:13 +0400

Quoted 18 lines

There are some objects that are common in all the places which are used to keep track of frag queues, they are:
* hash table
* LRU list
* rw lock
* rnd number for hash function
* the number of queues
* the amount of memory occupied by queues
* secret timer
Move all this stuff into one structure (struct inet_frags) to make it possible use them uniformly in the future. Like with the previous patch this mostly consists of hunks like
(quoted diff) +1 -1
-    write_lock(&ipfrag_lock);
+    write_lock(&ip4_frags.lock);

Quoted 5 lines

To address the issue with exporting the number of queues and the amount of memory occupied by queues outside the .c file they are declared in, I introduce a couple of helpers.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

'ipfrag_secret_timer' became unused, so I deleted it in this patch too.

Pavel Emelyanov

author

2007-10-12 13:12 UTC #5 lore raw

PATCH 3/9 Collect common sysctl variables together

In reply to Pavel Emelyanov

Some sysctl variables are used to tune the frag queues management and it will be useful to work with them in a common way in the future, so move them into one structure, moreover they are the same for all the frag management codes.

I don't place them in the existing inet_frags object, introduced in the previous patch for two reasons:

1. to keep them in the __read_mostly section;
2. not to export the whole inet_frags objects outside.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +8 -0

index d51f238..ada03ba 100644
@@ -20,6 +20,13 @@ struct inet_frag_queue {
 
 #define INETFRAGS_HASHSZ		64
 
+struct inet_frags_ctl {
+	int high_thresh;
+	int low_thresh;
+	int timeout;
+	int secret_interval;
+};
+
 struct inet_frags {
 	struct list_head	lru_list;
 	struct hlist_head	hash[INETFRAGS_HASHSZ];

@@ -28,6 +35,7 @@ struct inet_frags {
 	int			nqueues;
 	atomic_t		mem;
 	struct timer_list	secret_timer;
+	struct inet_frags_ctl	*ctl;
 };
 
 void inet_frags_init(struct inet_frags *);

include/net/ip.h +2 -4

index a18dcec..e7b0feb 100644
@@ -177,10 +177,8 @@ extern int sysctl_ip_default_ttl;
 extern int sysctl_ip_nonlocal_bind;
 
 /* From ip_fragment.c */
-extern int sysctl_ipfrag_high_thresh; 
-extern int sysctl_ipfrag_low_thresh;
-extern int sysctl_ipfrag_time;
-extern int sysctl_ipfrag_secret_interval;
+struct inet_frags_ctl;
+extern struct inet_frags_ctl ip4_frags_ctl;
 extern int sysctl_ipfrag_max_dist;
 
 /* From inetpeer.c */

include/net/ipv6.h +2 -4

index 77cdab3..b29d76c 100644
@@ -565,10 +565,8 @@ extern int inet6_hash_connect(struct inet_timewait_death_row *death_row,
 /*
  * reassembly.c
  */
-extern int sysctl_ip6frag_high_thresh;
-extern int sysctl_ip6frag_low_thresh;
-extern int sysctl_ip6frag_time;
-extern int sysctl_ip6frag_secret_interval;
+struct inet_frags_ctl;
+extern struct inet_frags_ctl ip6_frags_ctl;
 
 extern const struct proto_ops inet6_stream_ops;
 extern const struct proto_ops inet6_dgram_ops;

include/net/netfilter/ipv6/nf_conntrack_ipv6.h +2 -3

index 070d12c..f703533 100644
@@ -15,8 +15,7 @@ extern void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
 			       struct net_device *out,
 			       int (*okfn)(struct sk_buff *));
 
-extern unsigned int nf_ct_frag6_timeout;
-extern unsigned int nf_ct_frag6_low_thresh;
-extern unsigned int nf_ct_frag6_high_thresh;
+struct inet_frags_ctl;
+extern struct inet_frags_ctl nf_frags_ctl;
 
 #endif /* _NF_CONNTRACK_IPV6_H*/

net/ipv4/ip_fragment.c +26 ~~-21~~

105 lines not on this page · raw message

net/ipv4/sysctl_net_ipv4.c +5 -4

index eb286ab..c98ef16 100644
@@ -19,6 +19,7 @@
 #include <net/route.h>
 #include <net/tcp.h>
 #include <net/cipso_ipv4.h>
+#include <net/inet_frag.h>
 
 /* From af_inet.c */
 extern int sysctl_ip_nonlocal_bind;

@@ -357,7 +358,7 @@ ctl_table ipv4_table[] = {
 	{
 		.ctl_name	= NET_IPV4_IPFRAG_HIGH_THRESH,
 		.procname	= "ipfrag_high_thresh",
-		.data		= &sysctl_ipfrag_high_thresh,
+		.data		= &ip4_frags_ctl.high_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec

@@ -365,7 +366,7 @@ ctl_table ipv4_table[] = {
 	{
 		.ctl_name	= NET_IPV4_IPFRAG_LOW_THRESH,
 		.procname	= "ipfrag_low_thresh",
-		.data		= &sysctl_ipfrag_low_thresh,
+		.data		= &ip4_frags_ctl.low_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec

@@ -381,7 +382,7 @@ ctl_table ipv4_table[] = {
 	{
 		.ctl_name	= NET_IPV4_IPFRAG_TIME,
 		.procname	= "ipfrag_time",
-		.data		= &sysctl_ipfrag_time,
+		.data		= &ip4_frags_ctl.timeout,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec_jiffies,

@@ -732,7 +733,7 @@ ctl_table ipv4_table[] = {
 	{
 		.ctl_name	= NET_IPV4_IPFRAG_SECRET_INTERVAL,
 		.procname	= "ipfrag_secret_interval",
-		.data		= &sysctl_ipfrag_secret_interval,
+		.data		= &ip4_frags_ctl.secret_interval,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec_jiffies,

net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c +4 -3

index 37a3db9..572c0bc 100644
@@ -18,6 +18,7 @@
 #include <linux/icmp.h>
 #include <linux/sysctl.h>
 #include <net/ipv6.h>
+#include <net/inet_frag.h>
 
 #include <linux/netfilter_ipv6.h>
 #include <net/netfilter/nf_conntrack.h>

@@ -307,7 +308,7 @@ static ctl_table nf_ct_ipv6_sysctl_table[] = {
 	{
 		.ctl_name	= NET_NF_CONNTRACK_FRAG6_TIMEOUT,
 		.procname	= "nf_conntrack_frag6_timeout",
-		.data		= &nf_ct_frag6_timeout,
+		.data		= &nf_frags_ctl.timeout,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec_jiffies,

@@ -315,7 +316,7 @@ static ctl_table nf_ct_ipv6_sysctl_table[] = {
 	{
 		.ctl_name	= NET_NF_CONNTRACK_FRAG6_LOW_THRESH,
 		.procname	= "nf_conntrack_frag6_low_thresh",
-		.data		= &nf_ct_frag6_low_thresh,
+		.data		= &nf_frags_ctl.low_thresh,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,

@@ -323,7 +324,7 @@ static ctl_table nf_ct_ipv6_sysctl_table[] = {
 	{
 		.ctl_name	= NET_NF_CONNTRACK_FRAG6_HIGH_THRESH,
 		.procname	= "nf_conntrack_frag6_high_thresh",
-		.data		= &nf_ct_frag6_high_thresh,
+		.data		= &nf_frags_ctl.high_thresh,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,

net/ipv6/netfilter/nf_conntrack_reasm.c +15 ~~-14~~

index eb2ca1b..966a888 100644
@@ -49,10 +49,6 @@
 #define NF_CT_FRAG6_LOW_THRESH 196608  /* == 192*1024 */
 #define NF_CT_FRAG6_TIMEOUT IPV6_FRAG_TIMEOUT
 
-unsigned int nf_ct_frag6_high_thresh __read_mostly = 256*1024;
-unsigned int nf_ct_frag6_low_thresh __read_mostly = 192*1024;
-unsigned long nf_ct_frag6_timeout __read_mostly = IPV6_FRAG_TIMEOUT;
-
 struct nf_ct_frag6_skb_cb
 {
 	struct inet6_skb_parm	h;

@@ -74,6 +70,13 @@ struct nf_ct_frag6_queue
 	__u16			nhoffset;
 };
 
+struct inet_frags_ctl nf_frags_ctl __read_mostly = {
+	.high_thresh	 = 256 * 1024,
+	.low_thresh	 = 192 * 1024,
+	.timeout	 = IPV6_FRAG_TIMEOUT,
+	.secret_interval = 10 * 60 * HZ,
+};
+
 static struct inet_frags nf_frags;
 
 static __inline__ void __fq_unlink(struct nf_ct_frag6_queue *fq)

@@ -117,8 +120,6 @@ static unsigned int ip6qhashfn(__be32 id, struct in6_addr *saddr,
 	return c & (INETFRAGS_HASHSZ - 1);
 }
 
-int nf_ct_frag6_secret_interval = 10 * 60 * HZ;
-
 static void nf_ct_frag6_secret_rebuild(unsigned long dummy)
 {
 	unsigned long now = jiffies;

@@ -144,7 +145,7 @@ static void nf_ct_frag6_secret_rebuild(unsigned long dummy)
 	}
 	write_unlock(&nf_frags.lock);
 
-	mod_timer(&nf_frags.secret_timer, now + nf_ct_frag6_secret_interval);
+	mod_timer(&nf_frags.secret_timer, now + nf_frags_ctl.secret_interval);
 }
 
 /* Memory Tracking Functions. */

@@ -229,10 +230,10 @@ static void nf_ct_frag6_evictor(void)
 	unsigned int work;
 
 	work = atomic_read(&nf_frags.mem);
-	if (work <= nf_ct_frag6_low_thresh)
+	if (work <= nf_frags_ctl.low_thresh)
 		return;
 
-	work -= nf_ct_frag6_low_thresh;
+	work -= nf_frags_ctl.low_thresh;
 	while (work > 0) {
 		read_lock(&nf_frags.lock);
 		if (list_empty(&nf_frags.lru_list)) {

@@ -296,7 +297,7 @@ static struct nf_ct_frag6_queue *nf_ct_frag6_intern(unsigned int hash,
 #endif
 	fq = fq_in;
 
-	if (!mod_timer(&fq->q.timer, jiffies + nf_ct_frag6_timeout))
+	if (!mod_timer(&fq->q.timer, jiffies + nf_frags_ctl.timeout))
 		atomic_inc(&fq->q.refcnt);
 
 	atomic_inc(&fq->q.refcnt);

@@ -766,7 +767,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb)
 		goto ret_orig;
 	}
 
-	if (atomic_read(&nf_frags.mem) > nf_ct_frag6_high_thresh)
+	if (atomic_read(&nf_frags.mem) > nf_frags_ctl.high_thresh)
 		nf_ct_frag6_evictor();
 
 	fq = fq_find(fhdr->identification, &hdr->saddr, &hdr->daddr);

@@ -838,10 +839,10 @@ int nf_ct_frag6_kfree_frags(struct sk_buff *skb)
 int nf_ct_frag6_init(void)
 {
 	setup_timer(&nf_frags.secret_timer, nf_ct_frag6_secret_rebuild, 0);
-	nf_frags.secret_timer.expires = jiffies
-					   + nf_ct_frag6_secret_interval;
+	nf_frags.secret_timer.expires = jiffies + nf_frags_ctl.secret_interval;
 	add_timer(&nf_frags.secret_timer);
 
+	nf_frags.ctl = &nf_frags_ctl;
 	inet_frags_init(&nf_frags);
 
 	return 0;

@@ -852,6 +853,6 @@ void nf_ct_frag6_cleanup(void)
 	inet_frags_fini(&nf_frags);
 
 	del_timer(&nf_frags.secret_timer);
-	nf_ct_frag6_low_thresh = 0;
+	nf_frags_ctl.low_thresh = 0;
 	nf_ct_frag6_evictor();
 }

net/ipv6/reassembly.c +13 ~~-12~~

index 7b6315f..f0e22be 100644
@@ -55,11 +55,6 @@
 #include <net/addrconf.h>
 #include <net/inet_frag.h>
 
-int sysctl_ip6frag_high_thresh __read_mostly = 256*1024;
-int sysctl_ip6frag_low_thresh __read_mostly = 192*1024;
-
-int sysctl_ip6frag_time __read_mostly = IPV6_FRAG_TIMEOUT;
-
 struct ip6frag_skb_cb
 {
 	struct inet6_skb_parm	h;

@@ -86,6 +81,13 @@ struct frag_queue
 	__u16			nhoffset;
 };
 
+struct inet_frags_ctl ip6_frags_ctl __read_mostly = {
+	.high_thresh 	 = 256 * 1024,
+	.low_thresh	 = 192 * 1024,
+	.timeout	 = IPV6_FRAG_TIMEOUT,
+	.secret_interval = 10 * 60 * HZ,
+};
+
 static struct inet_frags ip6_frags;
 
 int ip6_frag_nqueues(void)

@@ -143,8 +145,6 @@ static unsigned int ip6qhashfn(__be32 id, struct in6_addr *saddr,
 	return c & (INETFRAGS_HASHSZ - 1);
 }
 
-int sysctl_ip6frag_secret_interval __read_mostly = 10 * 60 * HZ;
-
 static void ip6_frag_secret_rebuild(unsigned long dummy)
 {
 	unsigned long now = jiffies;

@@ -173,7 +173,7 @@ static void ip6_frag_secret_rebuild(unsigned long dummy)
 	}
 	write_unlock(&ip6_frags.lock);
 
-	mod_timer(&ip6_frags.secret_timer, now + sysctl_ip6frag_secret_interval);
+	mod_timer(&ip6_frags.secret_timer, now + ip6_frags_ctl.secret_interval);
 }
 
 /* Memory Tracking Functions. */

@@ -252,7 +252,7 @@ static void ip6_evictor(struct inet6_dev *idev)
 	struct list_head *tmp;
 	int work;
 
-	work = atomic_read(&ip6_frags.mem) - sysctl_ip6frag_low_thresh;
+	work = atomic_read(&ip6_frags.mem) - ip6_frags_ctl.low_thresh;
 	if (work <= 0)
 		return;

@@ -344,7 +344,7 @@ static struct frag_queue *ip6_frag_intern(struct frag_queue *fq_in)
 #endif
 	fq = fq_in;
 
-	if (!mod_timer(&fq->q.timer, jiffies + sysctl_ip6frag_time))
+	if (!mod_timer(&fq->q.timer, jiffies + ip6_frags_ctl.timeout))
 		atomic_inc(&fq->q.refcnt);
 
 	atomic_inc(&fq->q.refcnt);

@@ -727,7 +727,7 @@ static int ipv6_frag_rcv(struct sk_buff **skbp)
 		return 1;
 	}
 
-	if (atomic_read(&ip6_frags.mem) > sysctl_ip6frag_high_thresh)
+	if (atomic_read(&ip6_frags.mem) > ip6_frags_ctl.high_thresh)
 		ip6_evictor(ip6_dst_idev(skb->dst));
 
 	if ((fq = fq_find(fhdr->identification, &hdr->saddr, &hdr->daddr,

@@ -765,8 +765,9 @@ void __init ipv6_frag_init(void)
 
 	init_timer(&ip6_frags.secret_timer);
 	ip6_frags.secret_timer.function = ip6_frag_secret_rebuild;
-	ip6_frags.secret_timer.expires = jiffies + sysctl_ip6frag_secret_interval;
+	ip6_frags.secret_timer.expires = jiffies + ip6_frags_ctl.secret_interval;
 	add_timer(&ip6_frags.secret_timer);
 
+	ip6_frags.ctl = &ip6_frags_ctl;
 	inet_frags_init(&ip6_frags);
 }

net/ipv6/sysctl_net_ipv6.c +5 -4

index 3fb4427..97c425f 100644
@@ -12,6 +12,7 @@
 #include <net/ndisc.h>
 #include <net/ipv6.h>
 #include <net/addrconf.h>
+#include <net/inet_frag.h>
 
 #ifdef CONFIG_SYSCTL

@@ -41,7 +42,7 @@ static ctl_table ipv6_table[] = {
 	{
 		.ctl_name	= NET_IPV6_IP6FRAG_HIGH_THRESH,
 		.procname	= "ip6frag_high_thresh",
-		.data		= &sysctl_ip6frag_high_thresh,
+		.data		= &ip6_frags_ctl.high_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec

@@ -49,7 +50,7 @@ static ctl_table ipv6_table[] = {
 	{
 		.ctl_name	= NET_IPV6_IP6FRAG_LOW_THRESH,
 		.procname	= "ip6frag_low_thresh",
-		.data		= &sysctl_ip6frag_low_thresh,
+		.data		= &ip6_frags_ctl.low_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec

@@ -57,7 +58,7 @@ static ctl_table ipv6_table[] = {
 	{
 		.ctl_name	= NET_IPV6_IP6FRAG_TIME,
 		.procname	= "ip6frag_time",
-		.data		= &sysctl_ip6frag_time,
+		.data		= &ip6_frags_ctl.timeout,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec_jiffies,

@@ -66,7 +67,7 @@ static ctl_table ipv6_table[] = {
 	{
 		.ctl_name	= NET_IPV6_IP6FRAG_SECRET_INTERVAL,
 		.procname	= "ip6frag_secret_interval",
-		.data		= &sysctl_ip6frag_secret_interval,
+		.data		= &ip6_frags_ctl.secret_interval,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec_jiffies,

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:33 UTC #6 lore raw

Re: [PATCH 3/9] Collect common sysctl variables together

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:10:05 +0400

Quoted 6 lines

Some sysctl variables are used to tune the frag queues management and it will be useful to work with them in a common way in the future, so move them into one structure, moreover they are the same for all the frag management codes.

Quoted 7 lines

I don't place them in the existing inet_frags object, introduced in the previous patch for two reasons:
1. to keep them in the __read_mostly section;
2. not to export the whole inet_frags objects outside.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:15 UTC #7 lore raw

PATCH 4/9 Consolidate the xxx_frag_kill

In reply to Pavel Emelyanov

Since now all the xxx_frag_kill functions now work with the generic inet_frag_queue data type, this can be moved into a common place.

The xxx_unlink() code is moved as well.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +2 -0

index ada03ba..9902363 100644
@@ -41,4 +41,6 @@ struct inet_frags {
 void inet_frags_init(struct inet_frags *);
 void inet_frags_fini(struct inet_frags *);
 
+void inet_frag_kill(struct inet_frag_queue *q, struct inet_frags *f);
+
 #endif

net/ipv4/inet_fragment.c +23 -0

index 69623ff..534eaa8 100644
@@ -42,3 +42,26 @@ void inet_frags_fini(struct inet_frags *f)
 {
 }
 EXPORT_SYMBOL(inet_frags_fini);
+
+static inline void fq_unlink(struct inet_frag_queue *fq, struct inet_frags *f)
+{
+	write_lock(&f->lock);
+	hlist_del(&fq->list);
+	list_del(&fq->lru_list);
+	f->nqueues--;
+	write_unlock(&f->lock);
+}
+
+void inet_frag_kill(struct inet_frag_queue *fq, struct inet_frags *f)
+{
+	if (del_timer(&fq->timer))
+		atomic_dec(&fq->refcnt);
+
+	if (!(fq->last_in & COMPLETE)) {
+		fq_unlink(fq, f);
+		atomic_dec(&fq->refcnt);
+		fq->last_in |= COMPLETE;
+	}
+}
+
+EXPORT_SYMBOL(inet_frag_kill);

net/ipv4/ip_fragment.c +1 ~~-22~~

index 61035a8..5b376c4 100644
@@ -105,20 +105,6 @@ int ip_frag_mem(void)
 	return atomic_read(&ip4_frags.mem);
 }
 
-static __inline__ void __ipq_unlink(struct ipq *qp)
-{
-	hlist_del(&qp->q.list);
-	list_del(&qp->q.lru_list);
-	ip4_frags.nqueues--;
-}
-
-static __inline__ void ipq_unlink(struct ipq *ipq)
-{
-	write_lock(&ip4_frags.lock);
-	__ipq_unlink(ipq);
-	write_unlock(&ip4_frags.lock);
-}
-
 static unsigned int ipqhashfn(__be16 id, __be32 saddr, __be32 daddr, u8 prot)
 {
 	return jhash_3words((__force u32)id << 16 | prot,

@@ -219,14 +205,7 @@ static __inline__ void ipq_put(struct ipq *ipq, int *work)
  */
 static void ipq_kill(struct ipq *ipq)
 {
-	if (del_timer(&ipq->q.timer))
-		atomic_dec(&ipq->q.refcnt);
-
-	if (!(ipq->q.last_in & COMPLETE)) {
-		ipq_unlink(ipq);
-		atomic_dec(&ipq->q.refcnt);
-		ipq->q.last_in |= COMPLETE;
-	}
+	inet_frag_kill(&ipq->q, &ip4_frags);
 }
 
 /* Memory limiting on fragments.  Evictor trashes the oldest

net/ipv6/netfilter/nf_conntrack_reasm.c +1 ~~-22~~

index 966a888..2ebe515 100644
@@ -79,20 +79,6 @@ struct inet_frags_ctl nf_frags_ctl __read_mostly = {
 
 static struct inet_frags nf_frags;
 
-static __inline__ void __fq_unlink(struct nf_ct_frag6_queue *fq)
-{
-	hlist_del(&fq->q.list);
-	list_del(&fq->q.lru_list);
-	nf_frags.nqueues--;
-}
-
-static __inline__ void fq_unlink(struct nf_ct_frag6_queue *fq)
-{
-	write_lock(&nf_frags.lock);
-	__fq_unlink(fq);
-	write_unlock(&nf_frags.lock);
-}
-
 static unsigned int ip6qhashfn(__be32 id, struct in6_addr *saddr,
 			       struct in6_addr *daddr)
 {

@@ -213,14 +199,7 @@ static __inline__ void fq_put(struct nf_ct_frag6_queue *fq, unsigned int *work)
  */
 static __inline__ void fq_kill(struct nf_ct_frag6_queue *fq)
 {
-	if (del_timer(&fq->q.timer))
-		atomic_dec(&fq->q.refcnt);
-
-	if (!(fq->q.last_in & COMPLETE)) {
-		fq_unlink(fq);
-		atomic_dec(&fq->q.refcnt);
-		fq->q.last_in |= COMPLETE;
-	}
+	inet_frag_kill(&fq->q, &nf_frags);
 }
 
 static void nf_ct_frag6_evictor(void)

net/ipv6/reassembly.c +1 ~~-22~~

index f0e22be..57e32f4 100644
@@ -100,20 +100,6 @@ int ip6_frag_mem(void)
 	return atomic_read(&ip6_frags.mem);
 }
 
-static __inline__ void __fq_unlink(struct frag_queue *fq)
-{
-	hlist_del(&fq->q.list);
-	list_del(&fq->q.lru_list);
-	ip6_frags.nqueues--;
-}
-
-static __inline__ void fq_unlink(struct frag_queue *fq)
-{
-	write_lock(&ip6_frags.lock);
-	__fq_unlink(fq);
-	write_unlock(&ip6_frags.lock);
-}
-
 /*
  * callers should be careful not to use the hash value outside the ipfrag_lock
  * as doing so could race with ipfrag_hash_rnd being recalculated.

@@ -236,14 +222,7 @@ static __inline__ void fq_put(struct frag_queue *fq, int *work)
  */
 static __inline__ void fq_kill(struct frag_queue *fq)
 {
-	if (del_timer(&fq->q.timer))
-		atomic_dec(&fq->q.refcnt);
-
-	if (!(fq->q.last_in & COMPLETE)) {
-		fq_unlink(fq);
-		atomic_dec(&fq->q.refcnt);
-		fq->q.last_in |= COMPLETE;
-	}
+	inet_frag_kill(&fq->q, &ip6_frags);
 }
 
 static void ip6_evictor(struct inet6_dev *idev)

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:37 UTC #8 lore raw

Re: [PATCH 4/9] Consolidate the xxx_frag_kill

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:12:43 +0400

Quoted 7 lines

Since now all the xxx_frag_kill functions now work with the generic inet_frag_queue data type, this can be moved into a common place.
The xxx_unlink() code is moved as well.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:18 UTC #9 lore raw

PATCH 5/9 Consolidate xxx_the secret_rebuild

In reply to Pavel Emelyanov

This code works with the generic data types as well, so move this into inet_fragment.c

This move makes it possible to hide the secret_timer management and the secret_rebuild routine completely in the inet_fragment.c

Introduce the ->hashfn() callback in inet_frags() to get the hashfun for a given inet_frag_queue() object.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +2 -0

index 9902363..e374412 100644
@@ -36,6 +36,8 @@ struct inet_frags {
 	atomic_t		mem;
 	struct timer_list	secret_timer;
 	struct inet_frags_ctl	*ctl;
+
+	unsigned int		(*hashfn)(struct inet_frag_queue *);
 };
 
 void inet_frags_init(struct inet_frags *);

net/ipv4/inet_fragment.c +35 -0

index 534eaa8..ec10e05 100644
@@ -16,9 +16,38 @@
 #include <linux/module.h>
 #include <linux/timer.h>
 #include <linux/mm.h>
+#include <linux/random.h>
 
 #include <net/inet_frag.h>
 
+static void inet_frag_secret_rebuild(unsigned long dummy)
+{
+	struct inet_frags *f = (struct inet_frags *)dummy;
+	unsigned long now = jiffies;
+	int i;
+
+	write_lock(&f->lock);
+	get_random_bytes(&f->rnd, sizeof(u32));
+	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
+		struct inet_frag_queue *q;
+		struct hlist_node *p, *n;
+
+		hlist_for_each_entry_safe(q, p, n, &f->hash[i], list) {
+			unsigned int hval = f->hashfn(q);
+
+			if (hval != i) {
+				hlist_del(&q->list);
+
+				/* Relink to new hash chain. */
+				hlist_add_head(&q->list, &f->hash[hval]);
+			}
+		}
+	}
+	write_unlock(&f->lock);
+
+	mod_timer(&f->secret_timer, now + f->ctl->secret_interval);
+}
+
 void inet_frags_init(struct inet_frags *f)
 {
 	int i;

@@ -35,11 +64,17 @@ void inet_frags_init(struct inet_frags *f)
 	f->nqueues = 0;
 	atomic_set(&f->mem, 0);
 
+	init_timer(&f->secret_timer);
+	f->secret_timer.function = inet_frag_secret_rebuild;
+	f->secret_timer.data = (unsigned long)f;
+	f->secret_timer.expires = jiffies + f->ctl->secret_interval;
+	add_timer(&f->secret_timer);
 }
 EXPORT_SYMBOL(inet_frags_init);
 
 void inet_frags_fini(struct inet_frags *f)
 {
+	del_timer(&f->secret_timer);
 }
 EXPORT_SYMBOL(inet_frags_fini);

net/ipv4/ip_fragment.c +5 ~~-29~~

index 5b376c4..7aee137 100644
@@ -112,32 +112,12 @@ static unsigned int ipqhashfn(__be16 id, __be32 saddr, __be32 daddr, u8 prot)
 			    ip4_frags.rnd) & (INETFRAGS_HASHSZ - 1);
 }
 
-static void ipfrag_secret_rebuild(unsigned long dummy)
+static unsigned int ip4_hashfn(struct inet_frag_queue *q)
 {
-	unsigned long now = jiffies;
-	int i;
+	struct ipq *ipq;
 
-	write_lock(&ip4_frags.lock);
-	get_random_bytes(&ip4_frags.rnd, sizeof(u32));
-	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
-		struct ipq *q;
-		struct hlist_node *p, *n;
-
-		hlist_for_each_entry_safe(q, p, n, &ip4_frags.hash[i], q.list) {
-			unsigned int hval = ipqhashfn(q->id, q->saddr,
-						      q->daddr, q->protocol);
-
-			if (hval != i) {
-				hlist_del(&q->q.list);
-
-				/* Relink to new hash chain. */
-				hlist_add_head(&q->q.list, &ip4_frags.hash[hval]);
-			}
-		}
-	}
-	write_unlock(&ip4_frags.lock);
-
-	mod_timer(&ip4_frags.secret_timer, now + ip4_frags_ctl.secret_interval);
+	ipq = container_of(q, struct ipq, q);
+	return ipqhashfn(ipq->id, ipq->saddr, ipq->daddr, ipq->protocol);
 }
 
 /* Memory Tracking Functions. */

@@ -705,12 +685,8 @@ struct sk_buff *ip_defrag(struct sk_buff *skb, u32 user)
 
 void __init ipfrag_init(void)
 {
-	init_timer(&ip4_frags.secret_timer);
-	ip4_frags.secret_timer.function = ipfrag_secret_rebuild;
-	ip4_frags.secret_timer.expires = jiffies + ip4_frags_ctl.secret_interval;
-	add_timer(&ip4_frags.secret_timer);
-
 	ip4_frags.ctl = &ip4_frags_ctl;
+	ip4_frags.hashfn = ip4_hashfn;
 	inet_frags_init(&ip4_frags);
 }

net/ipv6/netfilter/nf_conntrack_reasm.c +5 ~~-29~~

index 2ebe515..a3aef38 100644
@@ -106,32 +106,12 @@ static unsigned int ip6qhashfn(__be32 id, struct in6_addr *saddr,
 	return c & (INETFRAGS_HASHSZ - 1);
 }
 
-static void nf_ct_frag6_secret_rebuild(unsigned long dummy)
+static unsigned int nf_hashfn(struct inet_frag_queue *q)
 {
-	unsigned long now = jiffies;
-	int i;
+	struct nf_ct_frag6_queue *nq;
 
-	write_lock(&nf_frags.lock);
-	get_random_bytes(&nf_frags.rnd, sizeof(u32));
-	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
-		struct nf_ct_frag6_queue *q;
-		struct hlist_node *p, *n;
-
-		hlist_for_each_entry_safe(q, p, n, &nf_frags.hash[i], q.list) {
-			unsigned int hval = ip6qhashfn(q->id,
-						       &q->saddr,
-						       &q->daddr);
-			if (hval != i) {
-				hlist_del(&q->q.list);
-				/* Relink to new hash chain. */
-				hlist_add_head(&q->q.list,
-					       &nf_frags.hash[hval]);
-			}
-		}
-	}
-	write_unlock(&nf_frags.lock);
-
-	mod_timer(&nf_frags.secret_timer, now + nf_frags_ctl.secret_interval);
+	nq = container_of(q, struct nf_ct_frag6_queue, q);
+	return ip6qhashfn(nq->id, &nq->saddr, &nq->daddr);
 }
 
 /* Memory Tracking Functions. */

@@ -817,11 +797,8 @@ int nf_ct_frag6_kfree_frags(struct sk_buff *skb)
 
 int nf_ct_frag6_init(void)
 {
-	setup_timer(&nf_frags.secret_timer, nf_ct_frag6_secret_rebuild, 0);
-	nf_frags.secret_timer.expires = jiffies + nf_frags_ctl.secret_interval;
-	add_timer(&nf_frags.secret_timer);
-
 	nf_frags.ctl = &nf_frags_ctl;
+	nf_frags.hashfn = nf_hashfn;
 	inet_frags_init(&nf_frags);
 
 	return 0;

@@ -831,7 +808,6 @@ void nf_ct_frag6_cleanup(void)
 {
 	inet_frags_fini(&nf_frags);
 
-	del_timer(&nf_frags.secret_timer);
 	nf_frags_ctl.low_thresh = 0;
 	nf_ct_frag6_evictor();
 }

net/ipv6/reassembly.c +5 ~~-32~~

index 57e32f4..7a357fd 100644
@@ -131,35 +131,12 @@ static unsigned int ip6qhashfn(__be32 id, struct in6_addr *saddr,
 	return c & (INETFRAGS_HASHSZ - 1);
 }
 
-static void ip6_frag_secret_rebuild(unsigned long dummy)
+static unsigned int ip6_hashfn(struct inet_frag_queue *q)
 {
-	unsigned long now = jiffies;
-	int i;
-
-	write_lock(&ip6_frags.lock);
-	get_random_bytes(&ip6_frags.rnd, sizeof(u32));
-	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
-		struct frag_queue *q;
-		struct hlist_node *p, *n;
-
-		hlist_for_each_entry_safe(q, p, n, &ip6_frags.hash[i], q.list) {
-			unsigned int hval = ip6qhashfn(q->id,
-						       &q->saddr,
-						       &q->daddr);
-
-			if (hval != i) {
-				hlist_del(&q->q.list);
-
-				/* Relink to new hash chain. */
-				hlist_add_head(&q->q.list,
-					       &ip6_frags.hash[hval]);
-
-			}
-		}
-	}
-	write_unlock(&ip6_frags.lock);
+	struct frag_queue *fq;
 
-	mod_timer(&ip6_frags.secret_timer, now + ip6_frags_ctl.secret_interval);
+	fq = container_of(q, struct frag_queue, q);
+	return ip6qhashfn(fq->id, &fq->saddr, &fq->daddr);
 }
 
 /* Memory Tracking Functions. */

@@ -742,11 +719,7 @@ void __init ipv6_frag_init(void)
 	if (inet6_add_protocol(&frag_protocol, IPPROTO_FRAGMENT) < 0)
 		printk(KERN_ERR "ipv6_frag_init: Could not register protocol\n");
 
-	init_timer(&ip6_frags.secret_timer);
-	ip6_frags.secret_timer.function = ip6_frag_secret_rebuild;
-	ip6_frags.secret_timer.expires = jiffies + ip6_frags_ctl.secret_interval;
-	add_timer(&ip6_frags.secret_timer);
-
 	ip6_frags.ctl = &ip6_frags_ctl;
+	ip6_frags.hashfn = ip6_hashfn;
 	inet_frags_init(&ip6_frags);
 }

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:38 UTC #10 lore raw

Re: [PATCH 5/9] Consolidate xxx_the secret_rebuild

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:16:25 +0400

Quoted 11 lines

This code works with the generic data types as well, so move this into inet_fragment.c
This move makes it possible to hide the secret_timer management and the secret_rebuild routine completely in the inet_fragment.c
Introduce the ->hashfn() callback in inet_frags() to get the hashfun for a given inet_frag_queue() object.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:23 UTC #11 lore raw

PATCH 6/9 Consolidate the xxx_frag_destroy

In reply to Pavel Emelyanov

To make in possible we need to know the exact frag queue size for inet_frags->mem management and two callbacks:

* to destoy the skb (optional, used in conntracks only)
* to free the queue itself (mandatory, but later I plan to move the allocation and the destruction of frag_queues into the common place, so this callback will most likely be optional too).

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +5 -0

index e374412..2dd1cd4 100644
@@ -33,16 +33,21 @@ struct inet_frags {
 	rwlock_t		lock;
 	u32			rnd;
 	int			nqueues;
+	int			qsize;
 	atomic_t		mem;
 	struct timer_list	secret_timer;
 	struct inet_frags_ctl	*ctl;
 
 	unsigned int		(*hashfn)(struct inet_frag_queue *);
+	void			(*destructor)(struct inet_frag_queue *);
+	void			(*skb_free)(struct sk_buff *);
 };
 
 void inet_frags_init(struct inet_frags *);
 void inet_frags_fini(struct inet_frags *);
 
 void inet_frag_kill(struct inet_frag_queue *q, struct inet_frags *f);
+void inet_frag_destroy(struct inet_frag_queue *q,
+				struct inet_frags *f, int *work);
 
 #endif

net/ipv4/inet_fragment.c +40 -0

index ec10e05..15fb2c4 100644
@@ -17,6 +17,8 @@
 #include <linux/timer.h>
 #include <linux/mm.h>
 #include <linux/random.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
 
 #include <net/inet_frag.h>

@@ -100,3 +102,41 @@ void inet_frag_kill(struct inet_frag_queue *fq, struct inet_frags *f)
 }
 
 EXPORT_SYMBOL(inet_frag_kill);
+
+static inline void frag_kfree_skb(struct inet_frags *f, struct sk_buff *skb,
+						int *work)
+{
+	if (work)
+		*work -= skb->truesize;
+
+	atomic_sub(skb->truesize, &f->mem);
+	if (f->skb_free)
+		f->skb_free(skb);
+	kfree_skb(skb);
+}
+
+void inet_frag_destroy(struct inet_frag_queue *q, struct inet_frags *f,
+					int *work)
+{
+	struct sk_buff *fp;
+
+	BUG_TRAP(q->last_in & COMPLETE);
+	BUG_TRAP(del_timer(&q->timer) == 0);
+
+	/* Release all fragment data. */
+	fp = q->fragments;
+	while (fp) {
+		struct sk_buff *xp = fp->next;
+
+		frag_kfree_skb(f, fp, work);
+		fp = xp;
+	}
+
+	if (work)
+		*work -= f->qsize;
+	atomic_sub(f->qsize, &f->mem);
+
+	f->destructor(q);
+
+}
+EXPORT_SYMBOL(inet_frag_destroy);

net/ipv4/ip_fragment.c +10 ~~-29~~

index 7aee137..a59ac39 100644
@@ -129,11 +129,13 @@ static __inline__ void frag_kfree_skb(struct sk_buff *skb, int *work)
 	kfree_skb(skb);
 }
 
-static __inline__ void frag_free_queue(struct ipq *qp, int *work)
+static __inline__ void ip4_frag_free(struct inet_frag_queue *q)
 {
-	if (work)
-		*work -= sizeof(struct ipq);
-	atomic_sub(sizeof(struct ipq), &ip4_frags.mem);
+	struct ipq *qp;
+
+	qp = container_of(q, struct ipq, q);
+	if (qp->peer)
+		inet_putpeer(qp->peer);
 	kfree(qp);
 }

@@ -150,34 +152,10 @@ static __inline__ struct ipq *frag_alloc_queue(void)
 
 /* Destruction primitives. */
 
-/* Complete destruction of ipq. */
-static void ip_frag_destroy(struct ipq *qp, int *work)
-{
-	struct sk_buff *fp;
-
-	BUG_TRAP(qp->q.last_in&COMPLETE);
-	BUG_TRAP(del_timer(&qp->q.timer) == 0);
-
-	if (qp->peer)
-		inet_putpeer(qp->peer);
-
-	/* Release all fragment data. */
-	fp = qp->q.fragments;
-	while (fp) {
-		struct sk_buff *xp = fp->next;
-
-		frag_kfree_skb(fp, work);
-		fp = xp;
-	}
-
-	/* Finally, release the queue descriptor itself. */
-	frag_free_queue(qp, work);
-}
-
 static __inline__ void ipq_put(struct ipq *ipq, int *work)
 {
 	if (atomic_dec_and_test(&ipq->q.refcnt))
-		ip_frag_destroy(ipq, work);
+		inet_frag_destroy(&ipq->q, &ip4_frags, work);
 }
 
 /* Kill ipq entry. It is not destroyed immediately,

@@ -687,6 +665,9 @@ void __init ipfrag_init(void)
 {
 	ip4_frags.ctl = &ip4_frags_ctl;
 	ip4_frags.hashfn = ip4_hashfn;
+	ip4_frags.destructor = ip4_frag_free;
+	ip4_frags.skb_free = NULL;
+	ip4_frags.qsize = sizeof(struct ipq);
 	inet_frags_init(&ip4_frags);
 }

net/ipv6/netfilter/nf_conntrack_reasm.c +13 ~~-31~~

75 lines not on this page · raw message

net/ipv6/reassembly.c +6 ~~-26~~

index 7a357fd..74b2113 100644
@@ -148,12 +148,9 @@ static inline void frag_kfree_skb(struct sk_buff *skb, int *work)
 	kfree_skb(skb);
 }
 
-static inline void frag_free_queue(struct frag_queue *fq, int *work)
+static void ip6_frag_free(struct inet_frag_queue *fq)
 {
-	if (work)
-		*work -= sizeof(struct frag_queue);
-	atomic_sub(sizeof(struct frag_queue), &ip6_frags.mem);
-	kfree(fq);
+	kfree(container_of(fq, struct frag_queue, q));
 }
 
 static inline struct frag_queue *frag_alloc_queue(void)

@@ -168,30 +165,10 @@ static inline struct frag_queue *frag_alloc_queue(void)
 
 /* Destruction primitives. */
 
-/* Complete destruction of fq. */
-static void ip6_frag_destroy(struct frag_queue *fq, int *work)
-{
-	struct sk_buff *fp;
-
-	BUG_TRAP(fq->q.last_in&COMPLETE);
-	BUG_TRAP(del_timer(&fq->q.timer) == 0);
-
-	/* Release all fragment data. */
-	fp = fq->q.fragments;
-	while (fp) {
-		struct sk_buff *xp = fp->next;
-
-		frag_kfree_skb(fp, work);
-		fp = xp;
-	}
-
-	frag_free_queue(fq, work);
-}
-
 static __inline__ void fq_put(struct frag_queue *fq, int *work)
 {
 	if (atomic_dec_and_test(&fq->q.refcnt))
-		ip6_frag_destroy(fq, work);
+		inet_frag_destroy(&fq->q, &ip6_frags, work);
 }
 
 /* Kill fq entry. It is not destroyed immediately,

@@ -721,5 +698,8 @@ void __init ipv6_frag_init(void)
 
 	ip6_frags.ctl = &ip6_frags_ctl;
 	ip6_frags.hashfn = ip6_hashfn;
+	ip6_frags.destructor = ip6_frag_free;
+	ip6_frags.skb_free = NULL;
+	ip6_frags.qsize = sizeof(struct frag_queue);
 	inet_frags_init(&ip6_frags);
 }

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:39 UTC #12 lore raw

Re: [PATCH 6/9] Consolidate the xxx_frag_destroy

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:21:06 +0400

Quoted 10 lines

To make in possible we need to know the exact frag queue size for inet_frags->mem management and two callbacks:
* to destoy the skb (optional, used in conntracks only)
* to free the queue itself (mandatory, but later I plan to move the allocation and the destruction of frag_queues into the common place, so this callback will most likely be optional too).
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:26 UTC #13 lore raw

PATCH 7/9 Consolidate the xxx_evictor

In reply to Pavel Emelyanov

The evictors collect some statistics for ipv4 and ipv6, so make it return the number of evicted queues and account them all at once in the caller.

The XXX_ADD_STATS_BH() macros are just for this case, but maybe there are places in code, that can make use of them as well.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +1 -0

index 2dd1cd4..cf583cf 100644
@@ -49,5 +49,6 @@ void inet_frags_fini(struct inet_frags *);
 void inet_frag_kill(struct inet_frag_queue *q, struct inet_frags *f);
 void inet_frag_destroy(struct inet_frag_queue *q,
 				struct inet_frags *f, int *work);
+int inet_frag_evictor(struct inet_frags *f);
 
 #endif

include/net/ip.h +1 -0

index e7b0feb..00ed4f3 100644
@@ -160,6 +160,7 @@ DECLARE_SNMP_STAT(struct ipstats_mib, ip_statistics);
 #define IP_INC_STATS(field)		SNMP_INC_STATS(ip_statistics, field)
 #define IP_INC_STATS_BH(field)		SNMP_INC_STATS_BH(ip_statistics, field)
 #define IP_INC_STATS_USER(field) 	SNMP_INC_STATS_USER(ip_statistics, field)
+#define IP_ADD_STATS_BH(field, val)	SNMP_ADD_STATS_BH(ip_statistics, field, val)
 DECLARE_SNMP_STAT(struct linux_mib, net_statistics);
 #define NET_INC_STATS(field)		SNMP_INC_STATS(net_statistics, field)
 #define NET_INC_STATS_BH(field)		SNMP_INC_STATS_BH(net_statistics, field)

include/net/ipv6.h +9 -0

index b29d76c..a0f1042 100644
@@ -120,12 +120,21 @@ extern int sysctl_mld_max_msf;
 	SNMP_INC_STATS##modifier(statname##_statistics, (field));	\
 })
 
+#define _DEVADD(statname, modifier, idev, field, val)			\
+({									\
+	struct inet6_dev *_idev = (idev);				\
+	if (likely(_idev != NULL))					\
+		SNMP_ADD_STATS##modifier((_idev)->stats.statname, (field), (val)); \
+	SNMP_ADD_STATS##modifier(statname##_statistics, (field), (val));\
+})
+
 /* MIBs */
 DECLARE_SNMP_STAT(struct ipstats_mib, ipv6_statistics);
 
 #define IP6_INC_STATS(idev,field)	_DEVINC(ipv6, , idev, field)
 #define IP6_INC_STATS_BH(idev,field)	_DEVINC(ipv6, _BH, idev, field)
 #define IP6_INC_STATS_USER(idev,field)	_DEVINC(ipv6, _USER, idev, field)
+#define IP6_ADD_STATS_BH(idev,field,val) _DEVADD(ipv6, _BH, idev, field, val)
 
 DECLARE_SNMP_STAT(struct icmpv6_mib, icmpv6_statistics);
 DECLARE_SNMP_STAT(struct icmpv6msg_mib, icmpv6msg_statistics);

net/ipv4/inet_fragment.c +32 -0

index 15fb2c4..484cf51 100644
@@ -140,3 +140,35 @@ void inet_frag_destroy(struct inet_frag_queue *q, struct inet_frags *f,
 
 }
 EXPORT_SYMBOL(inet_frag_destroy);
+
+int inet_frag_evictor(struct inet_frags *f)
+{
+	struct inet_frag_queue *q;
+	int work, evicted = 0;
+
+	work = atomic_read(&f->mem) - f->ctl->low_thresh;
+	while (work > 0) {
+		read_lock(&f->lock);
+		if (list_empty(&f->lru_list)) {
+			read_unlock(&f->lock);
+			break;
+		}
+
+		q = list_first_entry(&f->lru_list,
+				struct inet_frag_queue, lru_list);
+		atomic_inc(&q->refcnt);
+		read_unlock(&f->lock);
+
+		spin_lock(&q->lock);
+		if (!(q->last_in & COMPLETE))
+			inet_frag_kill(q, f);
+		spin_unlock(&q->lock);
+
+		if (atomic_dec_and_test(&q->refcnt))
+			inet_frag_destroy(q, f, &work);
+		evicted++;
+	}
+
+	return evicted;
+}
+EXPORT_SYMBOL(inet_frag_evictor);

net/ipv4/ip_fragment.c +4 ~~-26~~

index a59ac39..4ea7948 100644
@@ -171,33 +171,11 @@ static void ipq_kill(struct ipq *ipq)
  */
 static void ip_evictor(void)
 {
-	struct ipq *qp;
-	struct list_head *tmp;
-	int work;
-
-	work = atomic_read(&ip4_frags.mem) - ip4_frags_ctl.low_thresh;
-	if (work <= 0)
-		return;
-
-	while (work > 0) {
-		read_lock(&ip4_frags.lock);
-		if (list_empty(&ip4_frags.lru_list)) {
-			read_unlock(&ip4_frags.lock);
-			return;
-		}
-		tmp = ip4_frags.lru_list.next;
-		qp = list_entry(tmp, struct ipq, q.lru_list);
-		atomic_inc(&qp->q.refcnt);
-		read_unlock(&ip4_frags.lock);
+	int evicted;
 
-		spin_lock(&qp->q.lock);
-		if (!(qp->q.last_in&COMPLETE))
-			ipq_kill(qp);
-		spin_unlock(&qp->q.lock);
-
-		ipq_put(qp, &work);
-		IP_INC_STATS_BH(IPSTATS_MIB_REASMFAILS);
-	}
+	evicted = inet_frag_evictor(&ip4_frags);
+	if (evicted)
+		IP_ADD_STATS_BH(IPSTATS_MIB_REASMFAILS, evicted);
 }
 
 /*

net/ipv6/netfilter/nf_conntrack_reasm.c +1 ~~-28~~

index 785f5cd..862d089 100644
@@ -163,34 +163,7 @@ static __inline__ void fq_kill(struct nf_ct_frag6_queue *fq)
 
 static void nf_ct_frag6_evictor(void)
 {
-	struct nf_ct_frag6_queue *fq;
-	struct list_head *tmp;
-	unsigned int work;
-
-	work = atomic_read(&nf_frags.mem);
-	if (work <= nf_frags_ctl.low_thresh)
-		return;
-
-	work -= nf_frags_ctl.low_thresh;
-	while (work > 0) {
-		read_lock(&nf_frags.lock);
-		if (list_empty(&nf_frags.lru_list)) {
-			read_unlock(&nf_frags.lock);
-			return;
-		}
-		tmp = nf_frags.lru_list.next;
-		BUG_ON(tmp == NULL);
-		fq = list_entry(tmp, struct nf_ct_frag6_queue, q.lru_list);
-		atomic_inc(&fq->q.refcnt);
-		read_unlock(&nf_frags.lock);
-
-		spin_lock(&fq->q.lock);
-		if (!(fq->q.last_in&COMPLETE))
-			fq_kill(fq);
-		spin_unlock(&fq->q.lock);
-
-		fq_put(fq, &work);
-	}
+	inet_frag_evictor(&nf_frags);
 }
 
 static void nf_ct_frag6_expire(unsigned long data)

net/ipv6/reassembly.c +4 ~~-26~~

index 74b2113..454db16 100644
@@ -181,33 +181,11 @@ static __inline__ void fq_kill(struct frag_queue *fq)
 
 static void ip6_evictor(struct inet6_dev *idev)
 {
-	struct frag_queue *fq;
-	struct list_head *tmp;
-	int work;
-
-	work = atomic_read(&ip6_frags.mem) - ip6_frags_ctl.low_thresh;
-	if (work <= 0)
-		return;
-
-	while(work > 0) {
-		read_lock(&ip6_frags.lock);
-		if (list_empty(&ip6_frags.lru_list)) {
-			read_unlock(&ip6_frags.lock);
-			return;
-		}
-		tmp = ip6_frags.lru_list.next;
-		fq = list_entry(tmp, struct frag_queue, q.lru_list);
-		atomic_inc(&fq->q.refcnt);
-		read_unlock(&ip6_frags.lock);
-
-		spin_lock(&fq->q.lock);
-		if (!(fq->q.last_in&COMPLETE))
-			fq_kill(fq);
-		spin_unlock(&fq->q.lock);
+	int evicted;
 
-		fq_put(fq, &work);
-		IP6_INC_STATS_BH(idev, IPSTATS_MIB_REASMFAILS);
-	}
+	evicted = inet_frag_evictor(&ip6_frags);
+	if (evicted)
+		IP6_ADD_STATS_BH(idev, IPSTATS_MIB_REASMFAILS, evicted);
 }
 
 static void ip6_frag_expire(unsigned long data)

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:40 UTC #14 lore raw

Re: [PATCH 7/9] Consolidate the xxx_evictor

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:24:27 +0400

Quoted 9 lines

The evictors collect some statistics for ipv4 and ipv6, so make it return the number of evicted queues and account them all at once in the caller.
The XXX_ADD_STATS_BH() macros are just for this case, but maybe there are places in code, that can make use of them as well.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied, thanks!

Pavel Emelyanov

author

2007-10-12 13:30 UTC #15 lore raw

PATCH 8/9 Small cleanup for xxx_put after evictor consolidation

In reply to Pavel Emelyanov

After the evictor code is consolidated there is no need in passing the extra pointer to the xxx_put() functions.

The only place when it made sense was the evictor code itself.

Maybe this change must got with the previous (or with the next) patch, but I try to make them shorter as much as possible to simplify the review (but they are still large anyway), so this change goes in a separate patch.

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/ipv4/ip_fragment.c +5 -5

index a59ac39..4ea7948 100644
@@ -152,10 +152,10 @@ static __inline__ struct ipq *frag_alloc_queue(void)
 
 /* Destruction primitives. */
 
-static __inline__ void ipq_put(struct ipq *ipq, int *work)
+static __inline__ void ipq_put(struct ipq *ipq)
 {
 	if (atomic_dec_and_test(&ipq->q.refcnt))
-		inet_frag_destroy(&ipq->q, &ip4_frags, work);
+		inet_frag_destroy(&ipq->q, &ip4_frags, NULL);
 }
 
 /* Kill ipq entry. It is not destroyed immediately,

@@ -227,7 +205,7 @@ static void ip_expire(unsigned long arg)
 	}
 out:
 	spin_unlock(&qp->q.lock);
-	ipq_put(qp, NULL);
+	ipq_put(qp);
 }
 
 /* Creation primitives. */

@@ -257,7 +235,7 @@ static struct ipq *ip_frag_intern(struct ipq *qp_in)
 			atomic_inc(&qp->q.refcnt);
 			write_unlock(&ip4_frags.lock);
 			qp_in->q.last_in |= COMPLETE;
-			ipq_put(qp_in, NULL);
+			ipq_put(qp_in);
 			return qp;
 		}
 	}

@@ -652,7 +630,7 @@ struct sk_buff *ip_defrag(struct sk_buff *skb, u32 user)
 			ret = ip_frag_reasm(qp, dev);
 
 		spin_unlock(&qp->q.lock);
-		ipq_put(qp, NULL);
+		ipq_put(qp);
 		return ret;
 	}

net/ipv6/netfilter/nf_conntrack_reasm.c +6 -6

index 785f5cd..862d089 100644
@@ -147,10 +147,10 @@ static inline struct nf_ct_frag6_queue *frag_alloc_queue(void)
 
 /* Destruction primitives. */
 
-static __inline__ void fq_put(struct nf_ct_frag6_queue *fq, unsigned int *work)
+static __inline__ void fq_put(struct nf_ct_frag6_queue *fq)
 {
 	if (atomic_dec_and_test(&fq->q.refcnt))
-		inet_frag_destroy(&fq->q, &nf_frags, work);
+		inet_frag_destroy(&fq->q, &nf_frags, NULL);
 }
 
 /* Kill fq entry. It is not destroyed immediately,

@@ -206,7 +179,7 @@ static void nf_ct_frag6_expire(unsigned long data)
 
 out:
 	spin_unlock(&fq->q.lock);
-	fq_put(fq, NULL);
+	fq_put(fq);
 }
 
 /* Creation primitives. */

@@ -228,7 +201,7 @@ static struct nf_ct_frag6_queue *nf_ct_frag6_intern(unsigned int hash,
 			atomic_inc(&fq->q.refcnt);
 			write_unlock(&nf_frags.lock);
 			fq_in->q.last_in |= COMPLETE;
-			fq_put(fq_in, NULL);
+			fq_put(fq_in);
 			return fq;
 		}
 	}

@@ -719,7 +692,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb)
 	if (nf_ct_frag6_queue(fq, clone, fhdr, nhoff) < 0) {
 		spin_unlock(&fq->q.lock);
 		pr_debug("Can't insert skb to queue\n");
-		fq_put(fq, NULL);
+		fq_put(fq);
 		goto ret_orig;
 	}

@@ -730,7 +703,7 @@ struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb)
 	}
 	spin_unlock(&fq->q.lock);
 
-	fq_put(fq, NULL);
+	fq_put(fq);
 	return ret_skb;
 
 ret_orig:

net/ipv6/reassembly.c +5 -5

index 74b2113..454db16 100644
@@ -165,10 +165,10 @@ static inline struct frag_queue *frag_alloc_queue(void)
 
 /* Destruction primitives. */
 
-static __inline__ void fq_put(struct frag_queue *fq, int *work)
+static __inline__ void fq_put(struct frag_queue *fq)
 {
 	if (atomic_dec_and_test(&fq->q.refcnt))
-		inet_frag_destroy(&fq->q, &ip6_frags, work);
+		inet_frag_destroy(&fq->q, &ip6_frags, NULL);
 }
 
 /* Kill fq entry. It is not destroyed immediately,

@@ -246,7 +224,7 @@ out:
 	if (dev)
 		dev_put(dev);
 	spin_unlock(&fq->q.lock);
-	fq_put(fq, NULL);
+	fq_put(fq);
 }
 
 /* Creation primitives. */

@@ -270,7 +248,7 @@ static struct frag_queue *ip6_frag_intern(struct frag_queue *fq_in)
 			atomic_inc(&fq->q.refcnt);
 			write_unlock(&ip6_frags.lock);
 			fq_in->q.last_in |= COMPLETE;
-			fq_put(fq_in, NULL);
+			fq_put(fq_in);
 			return fq;
 		}
 	}

@@ -676,7 +654,7 @@ static int ipv6_frag_rcv(struct sk_buff **skbp)
 			ret = ip6_frag_reasm(fq, skbp, dev);
 
 		spin_unlock(&fq->q.lock);
-		fq_put(fq, NULL);
+		fq_put(fq);
 		return ret;
 	}

Signature · 1 line

1.5.3.4

David Miller

maintainer Applied

2007-10-15 09:41 UTC #16 lore raw

Re: [PATCH 8/9] Small cleanup for xxx_put after evictor consolidation

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:27:40 +0400

Quoted 11 lines

After the evictor code is consolidated there is no need in passing the extra pointer to the xxx_put() functions.
The only place when it made sense was the evictor code itself.
Maybe this change must got with the previous (or with the next) patch, but I try to make them shorter as much as possible to simplify the review (but they are still large anyway), so this change goes in a separate patch.
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.

Pavel Emelyanov

author

2007-10-12 13:31 UTC #17 lore raw

PATCH 9/9 Consolidate the xxx_put

In reply to Pavel Emelyanov

These ones use the generic data types too, so move them in one place.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_frag.h +6 -0

index cf583cf..911c2cd 100644
@@ -51,4 +51,10 @@ void inet_frag_destroy(struct inet_frag_queue *q,
 				struct inet_frags *f, int *work);
 int inet_frag_evictor(struct inet_frags *f);
 
+static inline void inet_frag_put(struct inet_frag_queue *q, struct inet_frags *f)
+{
+	if (atomic_dec_and_test(&q->refcnt))
+		inet_frag_destroy(q, f, NULL);
+}
+
 #endif

net/ipv4/ip_fragment.c +1 -2

index 4ea7948..4470a50 100644
@@ -154,8 +154,7 @@ static __inline__ struct ipq *frag_alloc_queue(void)
 
 static __inline__ void ipq_put(struct ipq *ipq)
 {
-	if (atomic_dec_and_test(&ipq->q.refcnt))
-		inet_frag_destroy(&ipq->q, &ip4_frags, NULL);
+	inet_frag_put(&ipq->q, &ip4_frags);
 }
 
 /* Kill ipq entry. It is not destroyed immediately,

net/ipv6/netfilter/nf_conntrack_reasm.c +1 -2

index 862d089..bf6f2f0 100644
@@ -149,8 +149,7 @@ static inline struct nf_ct_frag6_queue *frag_alloc_queue(void)
 
 static __inline__ void fq_put(struct nf_ct_frag6_queue *fq)
 {
-	if (atomic_dec_and_test(&fq->q.refcnt))
-		inet_frag_destroy(&fq->q, &nf_frags, NULL);
+	inet_frag_put(&fq->q, &nf_frags);
 }
 
 /* Kill fq entry. It is not destroyed immediately,

net/ipv6/reassembly.c +1 -2

index 454db16..a50fbea 100644
@@ -167,8 +167,7 @@ static inline struct frag_queue *frag_alloc_queue(void)
 
 static __inline__ void fq_put(struct frag_queue *fq)
 {
-	if (atomic_dec_and_test(&fq->q.refcnt))
-		inet_frag_destroy(&fq->q, &ip6_frags, NULL);
+	inet_frag_put(&fq->q, &ip6_frags);
 }
 
 /* Kill fq entry. It is not destroyed immediately,

Signature · 1 line

1.5.3.4

David Miller

maintainer

2007-10-15 09:42 UTC #18 lore raw

Re: [PATCH 9/9] Consolidate the xxx_put

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 17:29:01 +0400

Quoted 4 lines

These ones use the generic data types too, so move them in one place.
Signed-off-by Pavel Emelyanov <[email protected]>

Also applied, thanks!

David Miller

maintainer

2007-10-15 09:42 UTC #19 lore raw

In reply to Pavel Emelyanov

From: Pavel Emelyanov <[email protected]>
Date: Fri, 12 Oct 2007 16:55:10 +0400

Quoted 11 lines

Patrick recently pointed out, that there are three places that perform IP fragments management. In ipv4, ipv6 and in ip6 conntracks. Looks like these places can be a bit consolidated.
The proposal is to create a common structure inet_frag_queue to put common fields like list heads, refcounts etc in, and include it into the specific fragment queues. Then such objects like hash tables, lists, locks etc are moved to common place (struct inet_frags). At the end common code is moved to the net/ipv4/inet_fragment.c.

Quoted 5 lines

The inet_ prefix in file names, data structures and functions, and the code place (net/ipv4) was proposed by Alexey, but the exact names were selectd by me, so maybe there can be a better ones.
Signed-off-by Pavel Emelyanov <[email protected]>

Thank you for doing this work, I applied it all.

I had to decide whether to apply Herbert's recent patches first or your's, because either way there would be some conflicts to resolve.

I handled it the best I could, but it seems OK from here.