Quoting lu.zhip...@zte.com.cn (2017-10-25 20:42:10)
>
> >> + if (NO_ERROR == GetIfEntry(&aMib_ifrow)) {
>
>
> thanks,
>
> I found may be overflow using GetIfEntry,thus GetIfEntry2 instead of
> GetIfEntry.
I tried your fix-up patch at:
https://patch-diff.githubusercontent.com/raw/mdroth/qemu/pull/5.patch
but ran into the compile issue below in a Fedora 20 mingw64 build environment.
It looks like you need MIB_IF_ROW2 instead of MIB_IFROW2, and even then it
looks like the struct fields are different and need to be fixed up.
I'll keep the current patch as it stands, but please send a tested
standalone fix against the qga tree to the list so we can get this case
addressed before rc0. I would also suggest we disable w32 stats completely
if GetIfEntry2() isn't available, since dealing with overflows is painful
from a management perspective.
x86_64-w64-mingw32-gcc -I/home/mdroth/qemu-build-w64/qga -Iqga
-I/home/mdroth/w/qemu4.git/tcg -I/home/mdroth/w/qemu4.git/tcg/i386 -I.
-I/home/mdroth/w/qemu4.git -I/home/mdroth/w/qemu4.git/accel/tcg
-I/home/mdroth/w/qemu4.git/include
-I/usr/x86_64-w64-mingw32/sys-root/mingw/include/pixman-1
-I/home/mdroth/w/qemu4.git/dtc/libfdt -Werror -mms-bitfields
-I/usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0
-I/usr/x86_64-w64-mingw32/sys-root/mingw/lib/glib-2.0/include -m64 -mcx16
-mthreads -D__USE_MINGW_ANSI_STDIO=1 -DWIN32_LEAN_AND_MEAN -DWINVER=0x501
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -Wall -Wendif-labels
-Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration
-Wold-style-definition -Wtype-limits -fstack-protector-all -isystem
/home/mdroth/w/vss-win32/ -I/home/mdroth/w/qemu4.git/tests -I
qga/qapi-generated -MMD -MP -MT qga/service-win32.o -MF qga/service-win32.d -O2
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g -c -o qga/service-win32.o
/home/mdroth/w/qemu4.git/qga/service-win32.c
/home/mdroth/w/qemu4.git/qga/commands-win32.c: In function
'guest_get_network_stats':
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1200:20: error: 'MIB_IF_ROW2' has
no member named 'dwIndex'
a_mid_ifrow.dwIndex = if_index;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1202:42: error: 'MIB_IF_ROW2' has
no member named 'dwInOctets'
stats->rx_bytes = a_mid_ifrow.dwInOctets;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1203:44: error: 'MIB_IF_ROW2' has
no member named 'dwInUcastPkts'
stats->rx_packets = a_mid_ifrow.dwInUcastPkts;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1204:41: error: 'MIB_IF_ROW2' has
no member named 'dwInErrors'
stats->rx_errs = a_mid_ifrow.dwInErrors;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1205:44: error: 'MIB_IF_ROW2' has
no member named 'dwInDiscards'
stats->rx_dropped = a_mid_ifrow.dwInDiscards;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1206:42: error: 'MIB_IF_ROW2' has
no member named 'dwOutOctets'
stats->tx_bytes = a_mid_ifrow.dwOutOctets;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1207:44: error: 'MIB_IF_ROW2' has
no member named 'dwOutUcastPkts'
stats->tx_packets = a_mid_ifrow.dwOutUcastPkts;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1208:41: error: 'MIB_IF_ROW2' has
no member named 'dwOutErrors'
stats->tx_errs = a_mid_ifrow.dwOutErrors;
^
/home/mdroth/w/qemu4.git/qga/commands-win32.c:1209:44: error: 'MIB_IF_ROW2' has
no member named 'dwOutDiscards'
stats->tx_dropped = a_mid_ifrow.dwOutDiscards;
^
make: *** [qga/commands-win32.o] Error 1
make: *** Waiting for unfinished jobs....
>
>
> 为了让您的VPlat虚拟机故障和docker故障得到高效的处理，请上报故障到: $VPlat技术支
> 持。
>
> 芦志朋 luzhipeng
>
>
> IT开发工程师 IT Development Engineer
> 操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D Institute/
> System Product
>
>
> [cid] [cid]
> 四川省成都市天府大道中段800号
> E: lu.zhip...@zte.com.cn
> www.zte.com.cn
>
> 原始邮件
> 发件人： <mdr...@linux.vnet.ibm.com>;
> 收件人：芦志朋10108272;
> 抄送人： <qemu-devel@nongnu.org>;芦志朋10108272;
> 日期：2017年10月26日 09:00
> 主题：Re: [PATCH v7 RESEND] qga: Add support network interface statistics
> inguest-network-get-interfaces command
>
>
> Quoting ZhiPeng Lu (2017-09-12 03:54:26)
> > 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.
> >
> > Signed-off-by: ZhiPeng Lu <lu.zhip...@zte.com.cn>
>
> Thanks, applied to qga tree:
> https://github.com/mdroth/qemu/commits/qga
>
> There were a few issues that I noted below, but I tested on linux/windows
> and things seem to be working well so I fixed them up in my tree.
>
> >
> > ---
> > v1->v2:
> > - correct some spelling mistake and add the stats data to the
> > guest-network-get-interfaces command instead of adding a new command.
> > v2-v3:
> > - optimize function implementation
> > v3->v4:
> > - modify compile error
> > v4->v5:
> > - rename some temporary variables and add str_trim_off function for
> >
> calculating the space num in front of the string in
> guest_get_network_stats
> > v5->v6:
> > - use g_strchug instead of str_trim_off implemented by myself
> > v6->v7:
> > - add implementation for windows
> > ---
> > qga/commands-posix.c |
> 72 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > qga/commands-win32.c | 48 +++++++++++++++++++++++++++++++++++
> > qga/qapi-schema.json | 38 ++++++++++++++++++++++++++-
> > 3 files changed, 156 insertions(+), 2 deletions(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index ab0c63d..da5dba0 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -1643,6 +1643,65 @@ guest_find_interface(GuestNetworkInterfaceList *head,
> > return head;
> > }
> >
> > +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;
>
> Not sure if it affects behavior in practice, but getline() documentation
> states that *line is only allocated if *line == NULL *and* n == 0. So we
> should set n to 0 before each call to be safe.
>
> > + fp = fopen(devinfo, "r");
> > + if (!fp) {
> > + return -1;
> > + }
> > + name_len = strlen(name);
> > + while (getline(&line, &n, fp) != -1) {
>
> Since 'line' is being allocated by getline(), we need to free it after
> each call, or rely on it to realloc and then free that value before
> returning. The latter seems simpler in this case.
>
> > + 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;
> > + char *trim_line;
> > + trim_line = g_strchug(line);
> > + if (trim_line[0] == '\0') {
> > + continue;
> > + }
> > + colon = strchr(trim_line, ':');
> > + if (!colon) {
> > + continue;
> > + }
> > + if (colon - name_len == trim_line &&
> > + strncmp(trim_line, name, name_len) == 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");
>
> Reporting the specific interface name might be useful here.
>
> > + return -1;
> > +}
> > +
> > /*
> > * Build information about guest interfaces
> > */
> >
> @@ -1659,6 +1718,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;
> >
> @@ -1778,7 +1838,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/commands-win32.c b/qga/commands-win32.c
> > index 619dbd2..e891253 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -1152,6 +1152,42 @@ out:
> > }
> > #endif
> >
> > +static DWORD get_interface_index(const char *guid)
> > +{
> > + ULONG index;
> > + DWORD status;
> > + wchar_t wbuf[512];
> > + snwprintf(wbuf, sizeof(wbuf), L"\\device\\tcpip_%s", guid);
> > + wbuf[sizeof(wbuf) - 1] = 0;
>
> wchar_t can be larger than 1 byte, so using sizeof(wchar_t[]) as an
> index into a wchar_t[] can cause an overrun.
>
> > + status = GetAdapterIndex (wbuf, &index);
> > + if (status != NO_ERROR) {
> > + return (DWORD)~0;
> > + } else {
> > + return index;
> > + }
> > +}
> > +static int guest_get_network_stats(const char *name,
> > + GuestNetworkInterfaceStat *stats)
> > +{
> > + DWORD IfIndex = 0;
> > + MIB_IFROW aMib_ifrow;
> > + memset(&aMib_ifrow, 0, sizeof(aMib_ifrow));
> > + IfIndex = get_interface_index(name);
>
> Per CODING_STYLE variable names should be lowercase with "_" for
> separating words
>
> > + aMib_ifrow.dwIndex = IfIndex;
> > + if (NO_ERROR == GetIfEntry(&aMib_ifrow)) {
> > + stats->rx_bytes = aMib_ifrow.dwInOctets;
> > + stats->rx_packets = aMib_ifrow.dwInUcastPkts;
> > + stats->rx_errs = aMib_ifrow.dwInErrors;
> > + stats->rx_dropped = aMib_ifrow.dwInDiscards;
> > + stats->tx_bytes = aMib_ifrow.dwOutOctets;
> > + stats->tx_packets = aMib_ifrow.dwOutUcastPkts;
> > + stats->tx_errs = aMib_ifrow.dwOutErrors;
> > + stats->tx_dropped = aMib_ifrow.dwOutDiscards;
> > + return 0;
> > + }
> > + return -1;
> > +}
> > +
> > GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
> > {
> > IP_ADAPTER_ADDRESSES *adptr_addrs, *addr;
> >
> @@ -1159,6 +1195,7 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces
> (Error **errp)
> > GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
> > GuestIpAddressList *head_addr, *cur_addr;
> > GuestNetworkInterfaceList *info;
> > + GuestNetworkInterfaceStat *interface_stat = NULL;
> > GuestIpAddressList *address_item = NULL;
> > unsigned char *mac_addr;
> > char *addr_str;
> >
> @@ -1238,6 +1275,17 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces
> (Error **errp)
> > info->value->has_ip_addresses = true;
> > info->value->ip_addresses = head_addr;
> > }
> > + if (!info->value->has_statistics) {
> > + interface_stat = g_malloc0(sizeof(*interface_stat));
> > + if (guest_get_network_stats(addr->AdapterName,
> > + interface_stat) == -1) {
> > + info->value->has_statistics = false;
> > + g_free(interface_stat);
> > + } else {
> > + info->value->statistics = interface_stat;
> > + info->value->has_statistics = true;
> > + }
> > + }
> > }
> > WSACleanup();
> > out:
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index 90a0c86..17884c7 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -643,6 +643,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.11
> > +##
> > +{ '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
> > @@ -651,12 +683,16 @@
> > #
> > # @ip-addresses: List of addresses assigned to @name
> > #
> > +# @statistics: various statistic counters related to @name
> > +# (since 2.11)
> > +#
> > # 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
> >
>
>
