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

Cyrill Gorcunov gorcunov at virtuozzo.com
Sat Jul 18 00:40:23 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 counters
   * setup VE start/stop hooks to allocate 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-34837

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>
---

Please simply give it a glance on idea, I've not yet tested this code well.

 include/linux/vznetstat.h           |   11 --
 kernel/ve/vznetstat/ip6_vznetstat.c |   90 +-----------------------
 kernel/ve/vznetstat/ip_vznetstat.c  |  135 ++----------------------------------
 kernel/ve/vznetstat/vznetstat.c     |   76 ++++++++++++++++++--
 4 files changed, 88 insertions(+), 224 deletions(-)

Index: linux-pcs7.git/include/linux/vznetstat.h
===================================================================
--- linux-pcs7.git.orig/include/linux/vznetstat.h
+++ linux-pcs7.git/include/linux/vznetstat.h
@@ -68,8 +68,6 @@ void venet_acct_classify_add_outgoing_pl
 		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) { }
@@ -86,15 +84,6 @@ static inline void venet_acct_classify_a
 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,28 @@ 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,33 @@ 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)
+/* For ip6_vznetstat dependency */
+void ip_vznetstat_touch(void)
 {
-	if (env->stat) {
-		venet_acct_put_stat(env->stat);
-		env->stat = NULL;
-	}
 }
-EXPORT_SYMBOL(fini_venet_acct_ip_stat);
+EXPORT_SYMBOL(ip_vznetstat_touch);
 
-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
@@ -1079,11 +1079,72 @@ static struct file_operations proc_venet
         .release	= seq_release,
 };
 
-/*
- * ---------------------------------------------------------------------------
- * Initialization
- * ---------------------------------------------------------------------------
- */
+static int init_acct_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;
+	} else
+		venet_acct_get_stat(ve->stat);
+
+	__module_get(THIS_MODULE);
+	return 0;
+}
+
+static void fini_acct_hook(void *data)
+{
+	struct ve_struct *ve = (struct ve_struct *)data;
+
+	venet_acct_put_stat(ve->stat);
+	module_put(THIS_MODULE);
+}
+
+static struct ve_hook venet_acct_hook = {
+	.init		= init_acct_hook,
+	.fini		= fini_acct_hook,
+	.priority	= HOOK_PRIO_NET_ACCT,
+	.owner		= THIS_MODULE,
+};
+
+static int __init __init_venetstat(void)
+{
+	struct ve_struct *ve;
+
+	mutex_lock(&ve_list_lock);
+	for_each_ve(ve) {
+		ve->stat = venet_acct_find_create_stat(ve->veid);
+		if (!ve->stat) {
+			for_each_ve(ve) {
+				venet_acct_put_stat(ve->stat);
+				ve->stat = NULL;
+			}
+			mutex_unlock(&ve_list_lock);
+			return -ENOMEM;
+		}
+	}
+	mutex_unlock(&ve_list_lock);
+
+	ve_hook_register(VE_SS_CHAIN, &venet_acct_hook);
+	return 0;
+}
+
+static void __exit __exit_venetstat(void)
+{
+	struct ve_struct *ve;
+
+	mutex_lock(&ve_list_lock);
+	for_each_ve(ve) {
+		venet_acct_put_stat(ve->stat);
+		ve->stat = NULL;
+	}
+	mutex_unlock(&ve_list_lock);
+
+	ve_hook_unregister(&venet_acct_hook);
+}
 
 int __init venetstat_init(void)
 {
@@ -1095,6 +1156,10 @@ int __init venetstat_init(void)
 	for (i = 0; i < STAT_HASH_LEN; i++)
 		INIT_LIST_HEAD(stat_hash_list + i);
 
+	i = __init_venetstat();
+	if (i)
+		return i;
+
 #if CONFIG_PROC_FS
 	de = proc_create("venetstat", S_IFREG|S_IRUSR, proc_vz_dir,
 			&proc_venetstat_operations);
@@ -1113,6 +1178,7 @@ int __init venetstat_init(void)
 
 void __exit venetstat_exit(void)
 {
+	__exit_venetstat();
 	vzioctl_unregister(&tc_ioctl_info);
 	venet_acct_destroy_all_stat();
 



More information about the Devel mailing list