[Devel] [RFC rh7 v2] ve/vznetstat: Move VE networks statistics allocation into a commaon place
Konstantin Khorenko
khorenko at virtuozzo.com
Mon Jul 20 05:44:24 PDT 2015
Volodya, please review it with the priority.
--
Best regards,
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
On 07/20/2015 11:47 AM, Cyrill Gorcunov wrote:
> 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
>
> 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 | 13 ---
> kernel/ve/vznetstat/ip6_vznetstat.c | 91 +-----------------------
> kernel/ve/vznetstat/ip_vznetstat.c | 135 +-----------------------------------
> kernel/ve/vznetstat/vznetstat.c | 76 ++++++++++++++++++--
> 4 files changed, 85 insertions(+), 230 deletions(-)
>
> Index: linux-pcs7.git/include/linux/vznetstat.h
> ===================================================================
> --- linux-pcs7.git.orig/include/linux/vznetstat.h
> +++ linux-pcs7.git/include/linux/vznetstat.h
> @@ -66,10 +66,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 +82,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
> @@ -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;
> +
> + if (!ve->stat) {
> + ve->stat = venet_acct_find_create_stat(ve->veid);
> + if (!ve->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);
> + ve->stat = NULL;
> + module_put(THIS_MODULE);
> +}
> +
> +static struct ve_hook venet_acct_hook = {
> + .init = init_acct_hook,
> + .fini = fini_acct_hook,
> + .priority = HOOK_PRIO_NET,
> + .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();
>
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
>
More information about the Devel
mailing list