Skip to contentSkip to conversation

: tcp_proc_register simplification.

PATCH New Denis V. Lunev 1 participant 7 messages Apr 8, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover : tcp_proc_register simplification.

I have found that tcp_proc_register looks in the same way as udp_proc_register. So, I have ported improvements made in the commits 6f191efe48af62dd5f8917dd21d187f896cd6c81..4ad96d39a2d74c1b2e400b602da2594f5098fc26 to TCP.

#1 lore raw

PATCH 1/1 : No need to check afinfo != NULL in tcp_proc_(un)register.

tcp_proc_register/tcp_proc_unregister are called with a static pointer only.

---
 net/ipv4/tcp_ipv4.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
net/ipv4/tcp_ipv4.c +0 -4
index fc1b505..e5dd52b 100644
@@ -2264,8 +2264,6 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 	int rc = 0;
 	struct proc_dir_entry *p;
 
-	if (!afinfo)
-		return -EINVAL;
 	afinfo->seq_fops->owner		= afinfo->owner;
 	afinfo->seq_fops->open		= tcp_seq_open;
 	afinfo->seq_fops->read		= seq_read;
@@ -2282,8 +2280,6 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 
 void tcp_proc_unregister(struct net *net, struct tcp_seq_afinfo *afinfo)
 {
-	if (!afinfo)
-		return;
 	proc_net_remove(net, afinfo->name);
 	memset(afinfo->seq_fops, 0, sizeof(*afinfo->seq_fops));
 }
Signature · 1 line
1.5.3.rc5
#2 lore raw

PATCH 1/1 : Move seq_ops from tcp_iter_state to tcp_seq_afinfo.

In reply to Denis V. Lunev

No need to create seq_operations for each instance of 'netstat'.

---
 include/net/tcp.h   |    3 +--
 net/ipv4/tcp_ipv4.c |   14 ++++++++------
 net/ipv6/tcp_ipv6.c |    4 +++-
 3 files changed, 12 insertions(+), 9 deletions(-)
include/net/tcp.h +1 -2
index f9d6b89..a386bdd 100644
@@ -1324,8 +1324,8 @@ struct tcp_seq_afinfo {
 	struct module		*owner;
 	char			*name;
 	sa_family_t		family;
-	int			(*seq_show) (struct seq_file *m, void *v);
 	struct file_operations	*seq_fops;
+	struct seq_operations	seq_ops;
 };
 
 struct tcp_iter_state {
@@ -1334,7 +1334,6 @@ struct tcp_iter_state {
 	enum tcp_seq_states	state;
 	struct sock		*syn_wait_sk;
 	int			bucket, sbucket, num, uid;
-	struct seq_operations	seq_ops;
 };
 
 extern int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo);
net/ipv4/tcp_ipv4.c +8 -6
index e5dd52b..0d0e220 100644
@@ -2230,13 +2230,9 @@ static int tcp_seq_open(struct inode *inode, struct file *file)
 		goto out_kfree;
 
 	s->family		= afinfo->family;
-	s->seq_ops.start	= tcp_seq_start;
-	s->seq_ops.next		= tcp_seq_next;
-	s->seq_ops.show		= afinfo->seq_show;
-	s->seq_ops.stop		= tcp_seq_stop;
 	s->p.net                = net;
 
-	rc = seq_open(file, &s->seq_ops);
+	rc = seq_open(file, &afinfo->seq_ops);
 	if (rc)
 		goto out_put_net;
 	seq = file->private_data;
@@ -2270,6 +2266,10 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 	afinfo->seq_fops->llseek	= seq_lseek;
 	afinfo->seq_fops->release	= tcp_seq_release;
 
+	afinfo->seq_ops.start		= tcp_seq_start;
+	afinfo->seq_ops.next		= tcp_seq_next;
+	afinfo->seq_ops.stop		= tcp_seq_stop;
+
 	p = proc_net_fops_create(net, afinfo->name, S_IRUGO, afinfo->seq_fops);
 	if (p)
 		p->data = afinfo;
@@ -2415,8 +2415,10 @@ static struct tcp_seq_afinfo tcp4_seq_afinfo = {
 	.owner		= THIS_MODULE,
 	.name		= "tcp",
 	.family		= AF_INET,
-	.seq_show	= tcp4_seq_show,
 	.seq_fops	= &tcp4_seq_fops,
+	.seq_ops	= {
+		.show		= tcp4_seq_show,
+	},
 };
 
 static int tcp4_proc_init_net(struct net *net)
net/ipv6/tcp_ipv6.c +3 -1
index 378cc40..09d9184 100644
@@ -2126,8 +2126,10 @@ static struct tcp_seq_afinfo tcp6_seq_afinfo = {
 	.owner		= THIS_MODULE,
 	.name		= "tcp6",
 	.family		= AF_INET6,
-	.seq_show	= tcp6_seq_show,
 	.seq_fops	= &tcp6_seq_fops,
+	.seq_ops	= {
+		.show		= tcp6_seq_show,
+	},
 };
 
 int tcp6_proc_init(struct net *net)
Signature · 1 line
1.5.3.rc5
#3 lore raw

PATCH 1/1 : Place file operations directly into tcp_seq_afinfo.

In reply to Denis V. Lunev

No need to have separate never-used variable.

---
 include/net/tcp.h   |    2 +-
 net/ipv4/tcp_ipv4.c |   15 ++++++---------
 net/ipv6/tcp_ipv6.c |    2 --
 3 files changed, 7 insertions(+), 12 deletions(-)
include/net/tcp.h +1 -1
index a386bdd..23915f3 100644
@@ -1324,7 +1324,7 @@ struct tcp_seq_afinfo {
 	struct module		*owner;
 	char			*name;
 	sa_family_t		family;
-	struct file_operations	*seq_fops;
+	struct file_operations	seq_fops;
 	struct seq_operations	seq_ops;
 };
 
net/ipv4/tcp_ipv4.c +6 -9
index 287af10..ebef394 100644
@@ -2233,17 +2233,17 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 	int rc = 0;
 	struct proc_dir_entry *p;
 
-	afinfo->seq_fops->owner		= afinfo->owner;
-	afinfo->seq_fops->open		= tcp_seq_open;
-	afinfo->seq_fops->read		= seq_read;
-	afinfo->seq_fops->llseek	= seq_lseek;
-	afinfo->seq_fops->release	= seq_release_net;
+	afinfo->seq_fops.owner		= afinfo->owner;
+	afinfo->seq_fops.open		= tcp_seq_open;
+	afinfo->seq_fops.read		= seq_read;
+	afinfo->seq_fops.llseek		= seq_lseek;
+	afinfo->seq_fops.release	= seq_release_net;
 
 	afinfo->seq_ops.start		= tcp_seq_start;
 	afinfo->seq_ops.next		= tcp_seq_next;
 	afinfo->seq_ops.stop		= tcp_seq_stop;
 
-	p = proc_net_fops_create(net, afinfo->name, S_IRUGO, afinfo->seq_fops);
+	p = proc_net_fops_create(net, afinfo->name, S_IRUGO, &afinfo->seq_fops);
 	if (p)
 		p->data = afinfo;
 	else
@@ -2254,7 +2254,6 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 void tcp_proc_unregister(struct net *net, struct tcp_seq_afinfo *afinfo)
 {
 	proc_net_remove(net, afinfo->name);
-	memset(afinfo->seq_fops, 0, sizeof(*afinfo->seq_fops));
 }
 
 static void get_openreq4(struct sock *sk, struct request_sock *req,
@@ -2383,12 +2382,10 @@ out:
 	return 0;
 }
 
-static struct file_operations tcp4_seq_fops;
 static struct tcp_seq_afinfo tcp4_seq_afinfo = {
 	.owner		= THIS_MODULE,
 	.name		= "tcp",
 	.family		= AF_INET,
-	.seq_fops	= &tcp4_seq_fops,
 	.seq_ops	= {
 		.show		= tcp4_seq_show,
 	},
net/ipv6/tcp_ipv6.c +0 -2
index 09d9184..15c7044 100644
@@ -2121,12 +2121,10 @@ out:
 	return 0;
 }
 
-static struct file_operations tcp6_seq_fops;
 static struct tcp_seq_afinfo tcp6_seq_afinfo = {
 	.owner		= THIS_MODULE,
 	.name		= "tcp6",
 	.family		= AF_INET6,
-	.seq_fops	= &tcp6_seq_fops,
 	.seq_ops	= {
 		.show		= tcp6_seq_show,
 	},
Signature · 1 line
1.5.3.rc5
#4 lore raw

PATCH 1/1 : Replace struct net on tcp_iter_state with seq_net_private.

In reply to Denis V. Lunev
---
 include/net/tcp.h   |    2 +-
 net/ipv4/tcp_ipv4.c |   11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)
include/net/tcp.h +1 -1
index 723b368..f9d6b89 100644
@@ -1329,7 +1329,7 @@ struct tcp_seq_afinfo {
 };
 
 struct tcp_iter_state {
-	struct net              *net;
+	struct seq_net_private	p;
 	sa_family_t		family;
 	enum tcp_seq_states	state;
 	struct sock		*syn_wait_sk;
net/ipv4/tcp_ipv4.c +5 -6
index df89a56..fc1b505 100644
@@ -1955,7 +1955,7 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
 	struct hlist_node *node;
 	struct sock *sk = cur;
 	struct tcp_iter_state* st = seq->private;
-	struct net *net = st->net;
+	struct net *net = seq_file_net(seq);
 
 	if (!sk) {
 		st->bucket = 0;
@@ -2036,7 +2036,7 @@ static void *listening_get_idx(struct seq_file *seq, loff_t *pos)
 static void *established_get_first(struct seq_file *seq)
 {
 	struct tcp_iter_state* st = seq->private;
-	struct net *net = st->net;
+	struct net *net = seq_file_net(seq);
 	void *rc = NULL;
 
 	for (st->bucket = 0; st->bucket < tcp_hashinfo.ehash_size; ++st->bucket) {
@@ -2077,7 +2077,7 @@ static void *established_get_next(struct seq_file *seq, void *cur)
 	struct inet_timewait_sock *tw;
 	struct hlist_node *node;
 	struct tcp_iter_state* st = seq->private;
-	struct net *net = st->net;
+	struct net *net = seq_file_net(seq);
 
 	++st->num;
 
@@ -2234,7 +2234,7 @@ static int tcp_seq_open(struct inode *inode, struct file *file)
 	s->seq_ops.next		= tcp_seq_next;
 	s->seq_ops.show		= afinfo->seq_show;
 	s->seq_ops.stop		= tcp_seq_stop;
-	s->net                  = net;
+	s->p.net                = net;
 
 	rc = seq_open(file, &s->seq_ops);
 	if (rc)
@@ -2253,9 +2253,8 @@ out_kfree:
 static int tcp_seq_release(struct inode *inode, struct file *file)
 {
 	struct seq_file *seq = file->private_data;
-	struct tcp_iter_state *s = seq->private;
 
-	put_net(s->net);
+	put_net(seq_file_net(seq));
 	seq_release_private(inode, file);
 	return 0;
 }
Signature · 1 line
1.5.3.rc5
#5 lore raw

PATCH 1/1 : Cleanup /proc/tcp[6] creation/removal.

In reply to Denis V. Lunev

Replace seq_open with seq_open_net and remove tcp_seq_release completely. seq_release_net will do this job just fine.

---
 net/ipv4/tcp_ipv4.c |   41 +++++++----------------------------------
 1 files changed, 7 insertions(+), 34 deletions(-)
net/ipv4/tcp_ipv4.c +7 -34
index 0d0e220..287af10 100644
@@ -2212,46 +2212,19 @@ static void tcp_seq_stop(struct seq_file *seq, void *v)
 static int tcp_seq_open(struct inode *inode, struct file *file)
 {
 	struct tcp_seq_afinfo *afinfo = PDE(inode)->data;
-	struct seq_file *seq;
 	struct tcp_iter_state *s;
-	struct net *net;
-	int rc;
+	int err;
 
 	if (unlikely(afinfo == NULL))
 		return -EINVAL;
 
-	s = kzalloc(sizeof(*s), GFP_KERNEL);
-	if (!s)
-		return -ENOMEM;
-
-	rc = -ENXIO;
-	net = get_proc_net(inode);
-	if (!net)
-		goto out_kfree;
+	err = seq_open_net(inode, file, &afinfo->seq_ops,
+			  sizeof(struct tcp_iter_state));
+	if (err < 0)
+		return err;
 
+	s = ((struct seq_file *)file->private_data)->private;
 	s->family		= afinfo->family;
-	s->p.net                = net;
-
-	rc = seq_open(file, &afinfo->seq_ops);
-	if (rc)
-		goto out_put_net;
-	seq = file->private_data;
-	seq->private = s;
-out:
-	return rc;
-out_put_net:
-	put_net(net);
-out_kfree:
-	kfree(s);
-	goto out;
-}
-
-static int tcp_seq_release(struct inode *inode, struct file *file)
-{
-	struct seq_file *seq = file->private_data;
-
-	put_net(seq_file_net(seq));
-	seq_release_private(inode, file);
 	return 0;
 }
 
@@ -2264,7 +2237,7 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 	afinfo->seq_fops->open		= tcp_seq_open;
 	afinfo->seq_fops->read		= seq_read;
 	afinfo->seq_fops->llseek	= seq_lseek;
-	afinfo->seq_fops->release	= tcp_seq_release;
+	afinfo->seq_fops->release	= seq_release_net;
 
 	afinfo->seq_ops.start		= tcp_seq_start;
 	afinfo->seq_ops.next		= tcp_seq_next;
Signature · 1 line
1.5.3.rc5
#6 lore raw

PATCH 1/1 : Remove owner from tcp_seq_afinfo.

In reply to Denis V. Lunev

Move it to tcp_seq_afinfo->seq_fops as should be.

---
 include/net/tcp.h   |    1 -
 net/ipv4/tcp_ipv4.c |    5 +++--
 net/ipv6/tcp_ipv6.c |    4 +++-
 3 files changed, 6 insertions(+), 4 deletions(-)
include/net/tcp.h +0 -1
index 23915f3..2b273c0 100644
@@ -1321,7 +1321,6 @@ enum tcp_seq_states {
 };
 
 struct tcp_seq_afinfo {
-	struct module		*owner;
 	char			*name;
 	sa_family_t		family;
 	struct file_operations	seq_fops;
net/ipv4/tcp_ipv4.c +3 -2
index ebef394..5d11d25 100644
@@ -2233,7 +2233,6 @@ int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo)
 	int rc = 0;
 	struct proc_dir_entry *p;
 
-	afinfo->seq_fops.owner		= afinfo->owner;
 	afinfo->seq_fops.open		= tcp_seq_open;
 	afinfo->seq_fops.read		= seq_read;
 	afinfo->seq_fops.llseek		= seq_lseek;
@@ -2383,9 +2382,11 @@ out:
 }
 
 static struct tcp_seq_afinfo tcp4_seq_afinfo = {
-	.owner		= THIS_MODULE,
 	.name		= "tcp",
 	.family		= AF_INET,
+	.seq_fops	= {
+		.owner		= THIS_MODULE,
+	},
 	.seq_ops	= {
 		.show		= tcp4_seq_show,
 	},
net/ipv6/tcp_ipv6.c +3 -1
index 15c7044..9044e50 100644
@@ -2122,9 +2122,11 @@ out:
 }
 
 static struct tcp_seq_afinfo tcp6_seq_afinfo = {
-	.owner		= THIS_MODULE,
 	.name		= "tcp6",
 	.family		= AF_INET6,
+	.seq_fops	= {
+		.owner		= THIS_MODULE,
+	},
 	.seq_ops	= {
 		.show		= tcp6_seq_show,
 	},
Signature · 1 line
1.5.3.rc5