All of lore.kernel.org
 help / color / mirror / Atom feed
From: <lu.zhipeng@zte.com.cn>
To: marcandre.lureau@gmail.com
Cc: mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] 答复: Re:  [PATCH RESEND v6] qga: Add support networkinterface statistics in guest-network-get-interfaces command
Date: Wed, 5 Jul 2017 08:59:46 +0800 (CST)	[thread overview]
Message-ID: <201707050859469538829@zte.com.cn> (raw)

[-- Attachment #1: Type: text/plain, Size: 13380 bytes --]

Thanks for the review.




>Hi>On Tue, Jul 4, 2017 at 10:51 AM ZhiPeng Lu <lu.zhipeng@zte.com.cn> wrote:> >we can get the network interface statistics inside a virtual machine by> >guest-network-get-interfaces command. it is very useful for us to monitor> >and analyze network traffic.>>>>


>It's nicer if you give v1->v2->..->v6 change summary at each revision

it's a good  suggestion.




>> Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>>>--->>  qga/commands-posix.c | 80>> +++++++++++++++++++++++++++++++++++++++++++++++++++->>  qga/qapi-schema.json | 38 ++++++++++++++++++++++++->>  2 files changed, 116 insertions(+), 2 deletions(-)>>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c>> index 915df9e..233b024 100644>> --- a/qga/commands-posix.c>> +++ b/qga/commands-posix.c>> @@ -1638,6 +1638,73 @@ guest_find_interface(GuestNetworkInterfaceList>> *head,>>      return head>>  }>>>> +>> +static int str_trim_off(const char *s, int off, int lmt)>> +{>> +    for ( off < lmt ++off) {>> +        if (!isspace(s[off])) {>> +            break>> +        }>> +    }>> +    return off>> +}>> +>> +static int guest_get_network_stats(const char *name,>> +                       GuestNetworkInterfaceStat *stats)>> +{>> +    int name_len>> +    char const *devinfo = "/proc/net/dev">> +    FILE *fp>> +    char *line = NULL, *colon>> +    size_t n>> +    fp = fopen(devinfo, "r")>> +    if (!fp) {>>+        return -1>> +    }>> +    name_len = strlen(name)>> +    while (getline(&line, &n, fp) != -1) {>> +        long long dummy>> +        long long rx_bytes>> +        long long rx_packets>> +        long long rx_errs>> +        long long rx_dropped>> +        long long tx_bytes>> +        long long tx_packets>> +        long long tx_errs>> +        long long tx_dropped>>> Why have local variables, and not pass stats->filed directly?

if passing stats->filed directly ,stats->filed stores wrong data when it doesn't  find the network interface.


>>+        int trim_off>>+        colon = strchr(line, ':')>> +        if (!colon) {>> +            continue>> +        }>> +        trim_off = str_trim_off(line, 0, strlen(line))>>>Couldn't you  call g_strchomp() ?







i agreed,i will call g_strchomp().

>> +        if (colon - name_len - trim_off == line &&>>+           strncmp(line + trim_off, name, colon - line - trim_off) == 0) {>> +            if (sscanf(colon + 1,>> +                "%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld>> %lld %lld %lld %lld %lld",>> +                  &rx_bytes, &rx_packets, &rx_errs, &rx_dropped,>> +                  &dummy, &dummy, &dummy, &dummy,>> +                  &tx_bytes, &tx_packets, &tx_errs, &tx_dropped,>> +                  &dummy, &dummy, &dummy, &dummy) != 16) {>> +                continue>> +            }>> +            stats->rx_bytes = rx_bytes>>+            stats->rx_packets = rx_packets>> +           stats->rx_errs = rx_errs>> +            stats->rx_dropped = rx_dropped>> +            stats->tx_bytes = tx_bytes>> +            stats->tx_packets = tx_packets>> +            stats->tx_errs = tx_errs>> +            stats->tx_dropped = tx_dropped>> +            fclose(fp)>> +            return 0>> +        }>> +    }>> +    fclose(fp)>> +    g_debug("/proc/net/dev: Interface not found")>> +    return -1>> +}>> +>>  /*>>   * Build information about guest interfaces>>  */>> @@ -1654,6 +1721,7 @@ GuestNetworkInterfaceList>> *qmp_guest_network_get_interfaces(Error **errp)>>      for (ifa = ifap ifa ifa = ifa->ifa_next) {> >         GuestNetworkInterfaceList *info> >         GuestIpAddressList **address_list = NULL, *address_item = NULL>> +        GuestNetworkInterfaceStat  *interface_stat = NULL>>          char addr4[INET_ADDRSTRLEN]>>          char addr6[INET6_ADDRSTRLEN]>>          int sock>> @@ -1773,7 +1841,17 @@ GuestNetworkInterfaceList>> *qmp_guest_network_get_interfaces(Error **errp)>>>>          info->value->has_ip_addresses = true>>>> ->> +        if (!info->value->has_statistics) {>> +            interface_stat = g_malloc0(sizeof(*interface_stat))>> +            if (guest_get_network_stats(info->value->name,>> +                interface_stat) == -1) {>> +                info->value->has_statistics = false>> +                g_free(interface_stat)>> +            } else {>> +                info->value->statistics = interface_stat>> +                info->value->has_statistics = true>> +            }>> +        }>>      }>>>>      freeifaddrs(ifap)>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json>> index a02dbf2..948219b 100644>> --- a/qga/qapi-schema.json>> +++ b/qga/qapi-schema.json>> @@ -635,6 +635,38 @@>>             'prefix': 'int'} }>>>>  ##>> +# @GuestNetworkInterfaceStat:>> +#>> +# @rx-bytes: total bytes received>> +#>> +# @rx-packets: total packets received>> +#>> +# @rx-errs: bad packets received>> +#>> +# @rx-dropped: receiver dropped packets>> +#>> +# @tx-bytes: total bytes transmitted>> +#>> +# @tx-packets: total packets transmitted>> +#>> +# @tx-errs: packet transmit problems>> +#>> +# @tx-dropped: dropped packets transmitted>> +#>> +# Since: 2.10>> +##>> +{ 'struct': 'GuestNetworkInterfaceStat',>> +  'data': {'rx-bytes': 'uint64',>> +            'rx-packets': 'uint64',>> +            'rx-errs': 'uint64',>> +            'rx-dropped': 'uint64',>> +            'tx-bytes': 'uint64',>> +            'tx-packets': 'uint64',>> +            'tx-errs': 'uint64',>> +            'tx-dropped': 'uint64'>> +           } }>> +>> +##>>  # @GuestNetworkInterface:>>  #>>  # @name: The name of interface for which info are being delivered>> @@ -643,12 +675,16 @@>>  #>>  # @ip-addresses: List of addresses assigned to @name>>  #>> +# @statistics: various statistic counters related to @name>> +# (since 2.10)>> +#>>  # Since: 1.1>>  ##>>  { 'struct': 'GuestNetworkInterface',>>    'data': {'name': 'str',>>             '*hardware-address': 'str',>> -           '*ip-addresses': ['GuestIpAddress'] } }>> +           '*ip-addresses': ['GuestIpAddress'],>> +           '*statistics': 'GuestNetworkInterfaceStat' } }>>>>  ##>>  # @guest-network-get-interfaces:>> -->> 1.8.3.1>>>looks good to me otherwise,>-- >Marc-André Lureau



















为了让您的VPlat虚拟化故障得到高效的处理,请上报故障到: $VPlat技术支持。


芦志朋 luzhipeng






IT开发工程师 IT Development
Engineer
操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R&D Institute/System Product









深圳市南山区科技南路55号中兴通讯研发大楼33楼 
33/F, R&D Building, ZTE
Corporation Hi-tech Road South, 
Hi-tech
Industrial Park Nanshan District, Shenzhen, P.R.China, 518057 
T: +86 755 xxxxxxxx F:+86 755 xxxxxxxx 
M: +86 xxxxxxxxxxx 
E: lu.zhipeng@zte.com.cn 
www.zte.com.cn






原始邮件



发件人: <marcandre.lureau@gmail.com>
收件人:芦志朋10108272 <mdroth@linux.vnet.ibm.com>
抄送人: <qemu-devel@nongnu.org>
日 期 :2017年07月04日 22:33
主 题 :Re: [Qemu-devel] [PATCH RESEND v6] qga: Add support networkinterface statistics in guest-network-get-interfaces command





Hi

On Tue, Jul 4, 2017 at 10:51 AM ZhiPeng Lu <lu.zhipeng@zte.com.cn> wrote:

> we can get the network interface statistics inside a virtual machine by
> guest-network-get-interfaces command. it is very useful for us to monitor
> and analyze network traffic.
>
>
It's nicer if you give v1->v2->..->v6 change summary at each revision


> Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
> ---
>  qga/commands-posix.c | 80
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qga/qapi-schema.json | 38 ++++++++++++++++++++++++-
>  2 files changed, 116 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 915df9e..233b024 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1638,6 +1638,73 @@ guest_find_interface(GuestNetworkInterfaceList
> *head,
>      return head
>  }
>
> +
> +static int str_trim_off(const char *s, int off, int lmt)
> +{
> +    for ( off < lmt ++off) {
> +        if (!isspace(s[off])) {
> +            break
> +        }
> +    }
> +    return off
> +}
> +
> +static int guest_get_network_stats(const char *name,
> +                       GuestNetworkInterfaceStat *stats)
> +{
> +    int name_len
> +    char const *devinfo = "/proc/net/dev"
> +    FILE *fp
> +    char *line = NULL, *colon
> +    size_t n
> +    fp = fopen(devinfo, "r")
> +    if (!fp) {
> +        return -1
> +    }
> +    name_len = strlen(name)
> +    while (getline(&line, &n, fp) != -1) {
> +        long long dummy
> +        long long rx_bytes
> +        long long rx_packets
> +        long long rx_errs
> +        long long rx_dropped
> +        long long tx_bytes
> +        long long tx_packets
> +        long long tx_errs
> +        long long tx_dropped
>

 Why have local variables, and not pass stats->filed directly?

+        int trim_off
> +        colon = strchr(line, ':')
> +        if (!colon) {
> +            continue
> +        }
> +        trim_off = str_trim_off(line, 0, strlen(line))
>

Couldn't you  call g_strchomp() ?


> +        if (colon - name_len - trim_off == line &&
> +           strncmp(line + trim_off, name, colon - line - trim_off) == 0) {
> +            if (sscanf(colon + 1,
> +                "%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld
> %lld %lld %lld %lld %lld",
> +                  &rx_bytes, &rx_packets, &rx_errs, &rx_dropped,
> +                  &dummy, &dummy, &dummy, &dummy,
> +                  &tx_bytes, &tx_packets, &tx_errs, &tx_dropped,
> +                  &dummy, &dummy, &dummy, &dummy) != 16) {
> +                continue
> +            }
> +            stats->rx_bytes = rx_bytes
> +            stats->rx_packets = rx_packets
> +            stats->rx_errs = rx_errs
> +            stats->rx_dropped = rx_dropped
> +            stats->tx_bytes = tx_bytes
> +            stats->tx_packets = tx_packets
> +            stats->tx_errs = tx_errs
> +            stats->tx_dropped = tx_dropped
> +            fclose(fp)
> +            return 0
> +        }
> +    }
> +    fclose(fp)
> +    g_debug("/proc/net/dev: Interface not found")
> +    return -1
> +}
> +
>  /*
>   * Build information about guest interfaces
>   */
> @@ -1654,6 +1721,7 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces(Error **errp)
>      for (ifa = ifap ifa ifa = ifa->ifa_next) {
>          GuestNetworkInterfaceList *info
>          GuestIpAddressList **address_list = NULL, *address_item = NULL
> +        GuestNetworkInterfaceStat  *interface_stat = NULL
>          char addr4[INET_ADDRSTRLEN]
>          char addr6[INET6_ADDRSTRLEN]
>          int sock
> @@ -1773,7 +1841,17 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces(Error **errp)
>
>          info->value->has_ip_addresses = true
>
> -
> +        if (!info->value->has_statistics) {
> +            interface_stat = g_malloc0(sizeof(*interface_stat))
> +            if (guest_get_network_stats(info->value->name,
> +                interface_stat) == -1) {
> +                info->value->has_statistics = false
> +                g_free(interface_stat)
> +            } else {
> +                info->value->statistics = interface_stat
> +                info->value->has_statistics = true
> +            }
> +        }
>      }
>
>      freeifaddrs(ifap)
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index a02dbf2..948219b 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -635,6 +635,38 @@
>             'prefix': 'int'} }
>
>  ##
> +# @GuestNetworkInterfaceStat:
> +#
> +# @rx-bytes: total bytes received
> +#
> +# @rx-packets: total packets received
> +#
> +# @rx-errs: bad packets received
> +#
> +# @rx-dropped: receiver dropped packets
> +#
> +# @tx-bytes: total bytes transmitted
> +#
> +# @tx-packets: total packets transmitted
> +#
> +# @tx-errs: packet transmit problems
> +#
> +# @tx-dropped: dropped packets transmitted
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'GuestNetworkInterfaceStat',
> +  'data': {'rx-bytes': 'uint64',
> +            'rx-packets': 'uint64',
> +            'rx-errs': 'uint64',
> +            'rx-dropped': 'uint64',
> +            'tx-bytes': 'uint64',
> +            'tx-packets': 'uint64',
> +            'tx-errs': 'uint64',
> +            'tx-dropped': 'uint64'
> +           } }
> +
> +##
>  # @GuestNetworkInterface:
>  #
>  # @name: The name of interface for which info are being delivered
> @@ -643,12 +675,16 @@
>  #
>  # @ip-addresses: List of addresses assigned to @name
>  #
> +# @statistics: various statistic counters related to @name
> +# (since 2.10)
> +#
>  # Since: 1.1
>  ##
>  { 'struct': 'GuestNetworkInterface',
>    'data': {'name': 'str',
>             '*hardware-address': 'str',
> -           '*ip-addresses': ['GuestIpAddress'] } }
> +           '*ip-addresses': ['GuestIpAddress'],
> +           '*statistics': 'GuestNetworkInterfaceStat' } }
>
>  ##
>  # @guest-network-get-interfaces:
> --
> 1.8.3.1
>

looks good to me otherwise,

-- 
Marc-André Lureau

[-- Attachment #2: 24242e5637af428891c4db731e7765ad.jpg --]
[-- Type: image/jpeg, Size: 2064 bytes --]

[-- Attachment #3: 9ae3e214c17d49ed935d87c674ba3ee2.jpg --]
[-- Type: image/jpeg, Size: 6015 bytes --]

                 reply	other threads:[~2017-07-05  6:37 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201707050859469538829@zte.com.cn \
    --to=lu.zhipeng@zte.com.cn \
    --cc=marcandre.lureau@gmail.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.