[Devel] [RFC rh7 v3] ve/vznetstat: Move VE networks statistics allocation into a commaon place

Cyrill Gorcunov gorcunov at virtuozzo.com
Mon Jul 20 13:53:57 PDT 2015


We have three modules serving network statictics:

 - vznetstat.ko
 - ip_vznetstat.ko
 - ip6_vznetstat.ko

where ip6_vznetstat depends on ip_vznetstat and both depends on general vznetstat
module. While vznetstat does are real work on network packets counting the
counters are allocated in somehow nontrivial way

 1) ip_vznetstat depends on ip6_vznetstat, and both require vznetstat module
 2) when ip_vznetstat loads it walks over all existing VE and allocates counters
    setting up a ve-startup hook to allocate counters for VE which will be
    started after
 3) in turn ip6_vznetstat module doesn't depend on ip_vznetstat (which does
    a real counters allocations, in commit 3ad7b9565dc63734 we fixed
    ip6_vznetstat so it allocates counters for newly created VEs if
    former ip_vznetstat get unloaded, but this doesn't take existing
    VEs into account)

This makes counters allocation code really hard to read. Lets do their
allocation in a different way

 - move counters allocation into general vznetstat module, upon its
   load it does:

   * walk over all existing running VEs and allocate/get counters
   * setup VE start/stop hooks to allocate/get counters for further
     VEs which be started after the module load

 - drop counters allocation from both ip6_vznetstat and ip_vznetstat,
   these modules simply register netfilter hooks for incoming/outgoing
   traffic and count packets

 - drop ip_vznetstat dependency on ip6_vznetstat, they are not bound
   anymore and each simply depends on general vznetstat module which
   allocates counters for them

https://jira.sw.ru/browse/PSBM-35011

v2:
 - don't forget to assign newly created counters into VEs

v3:
 - switch to per-net operaions: instead of using start/stop
   hooks we simply register net init/exit hooks which either
   create/get or put/destroy the statistics. Note there is
   no longer module-get/put, we rely on net namespace
   synchronization, thus one can call rmmod over all
   modules mentioned above even with running VEs.

Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
CC: Andrey Vagin <avagin at virtuozzo.com>
CC: Vladimir Davydov <vdavydov at virtuozzo.com>
CC: Konstantin Khorenko <khorenko at virtuozzo.com>
CC: Pavel Emelyanov <xemul at virtuozzo.com>
CC: Igor Sukhih <igor at parallels.com>
---
 include/linux/vznetstat.h           |   15 ----
 kernel/ve/vznetstat/ip6_vznetstat.c |   91 +-----------------------
 kernel/ve/vznetstat/ip_vznetstat.c  |  135 +-----------------------------------
 kernel/ve/vznetstat/vznetstat.c     |   44 +++++++++--
 4 files changed, 51 insertions(+), 234 deletions(-)

Index: linux-pcs7.git/include/linux/vznetstat.h
===================================================================
--- linux-pcs7.git.orig/include/linux/vznetstat.h
+++ linux-pcs7.git/include/linux/vznetstat.h
@@ -29,8 +29,6 @@ struct acct_stat {
 	struct acct_counter cnt[TC_CLASS_MAX][ACCT_MAX];
 };
 
-#define VE_NET_ACCT_V6  1
-
 struct venet_stat {
 	struct list_head list;
 	envid_t  veid;
@@ -66,10 +64,7 @@ void venet_acct_classify_add_incoming_pl
 		struct ve_addr_struct *src_addr, int data_size);
 void venet_acct_classify_add_outgoing_plain(struct venet_stat *stat,
 		struct ve_addr_struct *dst_addr, int data_size);
-void ip_vznetstat_touch(void);
 
-int init_venet_acct_ip_stat(struct ve_struct *env, struct venet_stat *stat);
-void fini_venet_acct_ip_stat(struct ve_struct *env);
 #else /* !CONFIG_VE_NETDEV_ACCOUNTING */
 static inline void venet_acct_get_stat(struct venet_stat *stat) { }
 static inline void venet_acct_put_stat(struct venet_stat *stat) { }
@@ -85,16 +80,6 @@ static inline void venet_acct_classify_a
 		struct ve_addr_struct *src_addr, int data_size) {}
 static inline void venet_acct_classify_add_outgoing_plain(struct venet_stat *stat,
 		struct ve_addr_struct *dst_addr, int data_size) {}
-static inline void ip_vznetstat_touch(void) {}
-
-static inline int init_venet_acct_ip_stat(struct ve_struct *env, struct venet_stat *stat)
-{
-	return 0;
-}
-static void fini_venet_acct_ip_stat(struct ve_struct *env)
-{
-}
-
 #endif /* CONFIG_VE_NETDEV_ACCOUNTING */
 
 #endif
Index: linux-pcs7.git/kernel/ve/vznetstat/ip6_vznetstat.c
===================================================================
--- linux-pcs7.git.orig/kernel/ve/vznetstat/ip6_vznetstat.c
+++ linux-pcs7.git/kernel/ve/vznetstat/ip6_vznetstat.c
@@ -72,104 +72,27 @@ static struct nf_hook_ops venet_acct_out
 	.priority	= NF_IP6_PRI_LAST,
 };
 
-static int init_venet_acct_ip6_stat(void *data)
-{
-	struct ve_struct *ve = (struct ve_struct *)data;
-
-	if (!ve->stat) {
-		ve->stat = venet_acct_find_create_stat(ve->veid);
-		if (!ve->stat)
-			return -ENODEV;
-	} else {
-		venet_acct_get_stat(ve->stat);
-	}
-
-	__module_get(THIS_MODULE);
-	set_bit(VE_NET_ACCT_V6, &ve->stat->flags);
-
-	return 0;
-}
-
-static void fini_venet_acct_ip6_stat(void *data)
-{
-	struct ve_struct *ve = (struct ve_struct *)data;
-
-	/* module was load after VE ? */
-	if (!ve->stat || !test_bit(VE_NET_ACCT_V6, &ve->stat->flags))
-		return;
-
-	clear_bit(VE_NET_ACCT_V6, &ve->stat->flags);
-	venet_acct_put_stat(ve->stat);
-	module_put(THIS_MODULE);
-}
-
-
-static int venet_acct_register_ip6_hooks(void *data)
+int __init ip6_venetstat_init(void)
 {
-	struct ve_struct *ve = (struct ve_struct *)data;
 	int ret;
 
-	if (!ve->stat)
-		return -ENODEV;
-
-	venet_acct_get_stat(ve->stat);
-
-	 /* Register hooks */
 	ret = nf_register_hook(&venet_acct_in_ops);
 	if (ret < 0)
-		goto out_free_stat;
+		return ret;
 
 	ret = nf_register_hook(&venet_acct_out_ops);
-	if (ret < 0)
-		goto out_hook_in;
-
-	set_bit(VE_NET_ACCT_V6, &ve->stat->flags);
-
-	return 0;
-
-out_hook_in:
-	nf_unregister_hook(&venet_acct_in_ops);
-out_free_stat:
-	venet_acct_put_stat(ve->stat);
-	return ret;
-}
-
-static void venet_acct_unregister_ip6_hooks(void *data)
-{
-	struct ve_struct *ve = (struct ve_struct *)data;
-
-	clear_bit(VE_NET_ACCT_V6, &ve->stat->flags);
-
-	nf_unregister_hook(&venet_acct_out_ops);
-	nf_unregister_hook(&venet_acct_in_ops);
-
-	venet_acct_put_stat(ve->stat);
-}
-
-static struct ve_hook venet_acct_hook_v6 = {
-	.init		= init_venet_acct_ip6_stat,
-	.fini		= fini_venet_acct_ip6_stat,
-	.priority	= HOOK_PRIO_NET_ACCT_V6,
-	.owner		= THIS_MODULE,
-};
-
-int __init ip6_venetstat_init(void)
-{
-	int ret;
-
-	ret = venet_acct_register_ip6_hooks(get_ve0());
-	if (ret < 0)
+	if (ret < 0) {
+		nf_unregister_hook(&venet_acct_in_ops);
 		return ret;
+	}
 
-	ve_hook_register(VE_SS_CHAIN, &venet_acct_hook_v6);
-	ip_vznetstat_touch();
 	return 0;
 }
 
 void __exit ip6_venetstat_exit(void)
 {
-	venet_acct_unregister_ip6_hooks(get_ve0());
-	ve_hook_unregister(&venet_acct_hook_v6);
+	nf_unregister_hook(&venet_acct_out_ops);
+	nf_unregister_hook(&venet_acct_in_ops);
 }
 
 module_init(ip6_venetstat_init);
Index: linux-pcs7.git/kernel/ve/vznetstat/ip_vznetstat.c
===================================================================
--- linux-pcs7.git.orig/kernel/ve/vznetstat/ip_vznetstat.c
+++ linux-pcs7.git/kernel/ve/vznetstat/ip_vznetstat.c
@@ -136,148 +136,27 @@ static struct nf_hook_ops venet_acct_out
 	.priority	= NF_IP_PRI_LAST,
 };
 
-int init_venet_acct_ip_stat(struct ve_struct *env, struct venet_stat *stat)
-{
-	if (env->stat) {
-		WARN(1, "ve_struct->stat is not NULL, but should be.\n");
-		return -EEXIST;
-	}
-
-	env->stat = stat;
-	venet_acct_get_stat(stat);
-
-	return 0;
-}
-EXPORT_SYMBOL(init_venet_acct_ip_stat);
-
-void fini_venet_acct_ip_stat(struct ve_struct *env)
-{
-	if (env->stat) {
-		venet_acct_put_stat(env->stat);
-		env->stat = NULL;
-	}
-}
-EXPORT_SYMBOL(fini_venet_acct_ip_stat);
-
-static int venet_acct_register_ip_hooks(void)
+int __init ip_venetstat_init(void)
 {
 	int ret;
 
-	 /* Register hooks */
 	ret = nf_register_hook(&venet_acct_in_ops);
 	if (ret < 0)
-		goto out_free_stat;
+		return ret;
 
 	ret = nf_register_hook(&venet_acct_out_ops);
-	if (ret < 0)
-		goto out_err_out_hook;
-
-	return 0;
-
-out_err_out_hook:
-	nf_unregister_hook(&venet_acct_in_ops);
-out_free_stat:
-	return ret;
-}
-
-static void venet_acct_unregister_ip_hooks(void)
-{
-	nf_unregister_hook(&venet_acct_out_ops);
-	nf_unregister_hook(&venet_acct_in_ops);
-}
-
-/* For ip6_vznetstat dependency */
-void ip_vznetstat_touch(void)
-{
-}
-EXPORT_SYMBOL(ip_vznetstat_touch);
-
-static int init_venet_acct_ip_stat_hook(void *data)
-{
-	struct ve_struct *ve = (struct ve_struct *)data;
-	struct venet_stat *stat;
-
-	if (!ve->stat) {
-		stat = venet_acct_find_create_stat(ve->veid);
-		if (!stat)
-			return -ENOMEM;
-		init_venet_acct_ip_stat(ve, stat);
-	} else
-		venet_acct_get_stat(ve->stat);
-
-	__module_get(THIS_MODULE);
-	return 0;
-}
-
-static void fini_venet_acct_ip_stat_hook(void *data)
-{
-	struct ve_struct *ve = (struct ve_struct *)data;
-
-	/* module was load after VE ? */
-	if (!ve->stat)
-		return;
-
-	venet_acct_put_stat(ve->stat);
-	module_put(THIS_MODULE);
-}
-
-static struct ve_hook venet_acct_ip_stat_hook = {
-	.init		= init_venet_acct_ip_stat_hook,
-	.fini		= fini_venet_acct_ip_stat_hook,
-	.priority	= HOOK_PRIO_NET_ACCT,
-	.owner		= THIS_MODULE,
-};
-
-/*
- * ---------------------------------------------------------------------------
- * Initialization
- * ---------------------------------------------------------------------------
- */
-
-int __init ip_venetstat_init(void)
-{
-	struct ve_struct *ve;
-	int ret = -ENOMEM;
-
-	mutex_lock(&ve_list_lock);
-	for_each_ve(ve) {
-		BUG_ON(ve->stat);
-		ve->stat = venet_acct_find_create_stat(ve->veid);
-		if (!ve->stat)
-			goto err_locked;
+	if (ret < 0) {
+		nf_unregister_hook(&venet_acct_in_ops);
+		return ret;
 	}
-	mutex_unlock(&ve_list_lock);
-
-	ret = venet_acct_register_ip_hooks();
-	if (ret < 0)
-		goto err;
 
-	ve_hook_register(VE_SS_CHAIN, &venet_acct_ip_stat_hook);
 	return 0;
-err:
-	mutex_lock(&ve_list_lock);
-err_locked:
-	for_each_ve(ve) {
-		venet_acct_put_stat(ve->stat);
-		ve->stat = NULL;
-	}
-	mutex_unlock(&ve_list_lock);
-	return ret;
 }
 
 void __exit ip_venetstat_exit(void)
 {
-	struct ve_struct *ve;
-
-	venet_acct_unregister_ip_hooks();
-	ve_hook_unregister(&venet_acct_ip_stat_hook);
-
-	mutex_lock(&ve_list_lock);
-	for_each_ve(ve) {
-		venet_acct_put_stat(ve->stat);
-		ve->stat = NULL;
-	}
-	mutex_unlock(&ve_list_lock);
+	nf_unregister_hook(&venet_acct_out_ops);
+	nf_unregister_hook(&venet_acct_in_ops);
 }
 
 #if defined(MODULE) && defined(VZ_AUDIT)
Index: linux-pcs7.git/kernel/ve/vznetstat/vznetstat.c
===================================================================
--- linux-pcs7.git.orig/kernel/ve/vznetstat/vznetstat.c
+++ linux-pcs7.git/kernel/ve/vznetstat/vznetstat.c
@@ -29,7 +29,6 @@
 #include <uapi/linux/vzctl_netstat.h>
 #include <uapi/linux/vzcalluser.h>
 
-
 /*
  * ---------------------------------------------------------------------------
  * Traffic classes storage
@@ -1079,15 +1078,41 @@ static struct file_operations proc_venet
         .release	= seq_release,
 };
 
-/*
- * ---------------------------------------------------------------------------
- * Initialization
- * ---------------------------------------------------------------------------
- */
+static int __net_init net_init_acct(struct net *net)
+{
+	struct ve_struct *ve = net->owner_ve;
+
+	if (!ve->stat) {
+		ve->stat = venet_acct_find_create_stat(ve->veid);
+		if (!ve->stat)
+			return -ENOMEM;
+	} else
+		venet_acct_get_stat(ve->stat);
+
+	return 0;
+}
+
+static void __net_exit net_exit_acct(struct net *net)
+{
+	struct ve_struct *ve = net->owner_ve;
+
+	if (ve->stat) {
+		venet_acct_put_stat(ve->stat);
+		if (atomic_read(&ve->stat->users) == 0) {
+			venet_acct_destroy_stat(ve->veid);
+			ve->stat = NULL;
+		}
+	}
+}
+
+static struct pernet_operations __net_initdata net_acct_ops = {
+	.init	= net_init_acct,
+	.exit	= net_exit_acct,
+};
 
 int __init venetstat_init(void)
 {
-	int i;
+	int i, ret;
 #if CONFIG_PROC_FS
 	struct proc_dir_entry *de;
 #endif
@@ -1095,6 +1120,10 @@ int __init venetstat_init(void)
 	for (i = 0; i < STAT_HASH_LEN; i++)
 		INIT_LIST_HEAD(stat_hash_list + i);
 
+	ret = register_pernet_subsys(&net_acct_ops);
+	if (ret)
+		return ret;
+
 #if CONFIG_PROC_FS
 	de = proc_create("venetstat", S_IFREG|S_IRUSR, proc_vz_dir,
 			&proc_venetstat_operations);
@@ -1113,6 +1142,7 @@ int __init venetstat_init(void)
 
 void __exit venetstat_exit(void)
 {
+	unregister_pernet_subsys(&net_acct_ops);
 	vzioctl_unregister(&tc_ioctl_info);
 	venet_acct_destroy_all_stat();
 



More information about the Devel mailing list