[Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Amos Kong]

We want to implement mac programming over macvtap through Libvirt. The
related rx-filter information of the nic contains main mac, rx-mode
items.

This patch adds a QMP event to notify management of rx-filter change,
and adds a monitor command for management to query rx-filter
information.

A flag for each nic is used to avoid events flooding, if management
doesn't query rx-filter after it receives one event, new events won't
be emitted to QMP monitor.

There maybe exist an uncontrollable delay, guests normally expect
rx-filter updates immediately, but it's another separate issue, we
can investigate it after Libvirt work is done.

Signed-off-by: Amos Kong <akong@redhat.com>
---
v2: add argument to filter mac-table info of single nic (Stefan)
    update the document, add event notification
v3: rename to rx-filter, add main mac, avoid events flooding (MST)
    fix error process (Stefan), fix qmp interface (Eric)
v4: process qerror in hmp, cleanup (Luiz)
    set flag for each device, add device path in event, add
    helper for g_strdup_printf (MST)
    fix qmp document (Eric)
---
 QMP/qmp-events.txt        | 19 ++++++++++
 hmp-commands.hx           |  2 ++
 hmp.c                     | 48 +++++++++++++++++++++++++
 hmp.h                     |  1 +
 hw/net/virtio-net.c       | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 include/monitor/monitor.h |  1 +
 include/net/net.h         |  3 ++
 monitor.c                 |  9 +++++
 net/net.c                 | 47 +++++++++++++++++++++++++
 qapi-schema.json          | 72 ++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx           | 58 ++++++++++++++++++++++++++++++
 11 files changed, 349 insertions(+)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 92fe5fb..315bbf8 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -172,6 +172,25 @@ Data:
   },
   "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
 
+NIC_RX_FILTER_CHANGED
+-----------------
+
+Emitted when rx-filter configuration of nic is changed by the guest.
+Each nic has a flag to control event emit, the flag is set to false
+when it emits one event of the nic, the flag is set to true when
+management queries the rx-filter of the nic. This is used to avoid
+events flooding.
+
+Data:
+
+- "name": net client name (json-string)
+
+{ "event": "NIC_RX_FILTER_CHANGED",
+  "data": { "name": "vnet0",
+            "path": "/machine/peripheral/vnet0/virtio-backend" },
+  "timestamp": { "seconds": 1368697518, "microseconds": 326866 } }
+}
+
 RESET
 -----
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9cea415..b7863eb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1639,6 +1639,8 @@ show qdev device model list
 show roms
 @item info tpm
 show the TPM device
+@item info rx-filter [net client name]
+show the rx-filter information for all nics (or for the given nic)
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index 4fb76ec..af04eb1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -653,6 +653,54 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
     qapi_free_TPMInfoList(info_list);
 }
 
+static void strlist_printf(Monitor *mon, strList *entry)
+{
+    while (entry) {
+        monitor_printf(mon, "    %s\n", entry->value);
+        entry = entry->next;
+    }
+}
+
+void hmp_info_rx_filter(Monitor *mon, const QDict *qdict)
+{
+    RxFilterInfoList *filter_list, *entry;
+    bool has_name = qdict_haskey(qdict, "name");
+    const char *name = NULL;
+    Error *errp = NULL;
+
+    if (has_name) {
+        name = qdict_get_str(qdict, "name");
+    }
+
+    filter_list = entry = qmp_query_rx_filter(has_name, name, &errp);
+    hmp_handle_error(mon, &errp);
+
+    while (entry) {
+        monitor_printf(mon, "%s:\n", entry->value->name);
+        monitor_printf(mon, " \\ promiscuous: %s\n",
+                       entry->value->promiscuous ? "on" : "off");
+        monitor_printf(mon, " \\ multicast: %s\n",
+                       RxState_lookup[entry->value->multicast]);
+        monitor_printf(mon, " \\ unicast: %s\n",
+                       RxState_lookup[entry->value->unicast]);
+        monitor_printf(mon, " \\ broadcast-allowed: %s\n",
+                       entry->value->broadcast_allowed ? "on" : "off");
+        monitor_printf(mon, " \\ multicast-overflow: %s\n",
+                       entry->value->multicast_overflow ? "on" : "off");
+        monitor_printf(mon, " \\ unicast-overflow: %s\n",
+                       entry->value->unicast_overflow ? "on" : "off");
+        monitor_printf(mon, " \\ main-mac: %s\n", entry->value->main_mac);
+
+        monitor_printf(mon, " \\ unicast-table:\n");
+        strlist_printf(mon, entry->value->unicast_table);
+        monitor_printf(mon, " \\ multicast-table:\n");
+        strlist_printf(mon, entry->value->multicast_table);
+
+        entry = entry->next;
+    }
+    qapi_free_RxFilterInfoList(filter_list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 95fe76e..9af733e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_rx_filter(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 1ea9556..56df052 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -21,6 +21,8 @@
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
 #include "hw/virtio/virtio-bus.h"
+#include "qapi/qmp/qjson.h"
+#include "monitor/monitor.h"
 
 #define VIRTIO_NET_VM_VERSION    11
 
@@ -192,6 +194,84 @@ static void virtio_net_set_link_status(NetClientState *nc)
     virtio_net_set_status(vdev, vdev->status);
 }
 
+static void rxfilter_notify(NetClientState *nc)
+{
+    QObject *event_data;
+    VirtIONet *n = qemu_get_nic_opaque(nc);
+
+    if (nc->rxfilter_notify_enabled) {
+        event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
+                           n->netclient_name,
+                           object_get_canonical_path(OBJECT(n->qdev)));
+        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
+        qobject_decref(event_data);
+        /* disable event notification to avoid events flooding */
+        nc->rxfilter_notify_enabled = false;
+    }
+}
+
+static char *mac_strdup_printf(uint8_t *mac)
+{
+    return g_strdup_printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x", mac[0],
+                            mac[1], mac[2], mac[3], mac[4], mac[5]);
+}
+
+static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
+{
+    VirtIONet *n = qemu_get_nic_opaque(nc);
+    RxFilterInfo *info;
+    strList *str_list = NULL;
+    strList *entry;
+    int i;
+
+    info = g_malloc0(sizeof(*info));
+    info->name = g_strdup(nc->name);
+    info->promiscuous = n->promisc;
+
+    if (n->nouni) {
+        info->unicast = RX_STATE_NONE;
+    } else if (n->alluni) {
+        info->unicast = RX_STATE_ALL;
+    } else {
+        info->unicast = RX_STATE_NORMAL;
+    }
+
+    if (n->nomulti) {
+        info->multicast = RX_STATE_NONE;
+    } else if (n->allmulti) {
+        info->multicast = RX_STATE_ALL;
+    } else {
+        info->multicast = RX_STATE_NORMAL;
+    }
+
+    info->broadcast_allowed = n->nobcast;
+    info->multicast_overflow = n->mac_table.multi_overflow;
+    info->unicast_overflow = n->mac_table.uni_overflow;
+
+    info->main_mac = mac_strdup_printf(n->mac);
+
+    for (i = 0; i < n->mac_table.first_multi; i++) {
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
+        entry->next = str_list;
+        str_list = entry;
+    }
+    info->unicast_table = str_list;
+
+    str_list = NULL;
+    for (i = n->mac_table.first_multi; i < n->mac_table.in_use; i++) {
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
+        entry->next = str_list;
+        str_list = entry;
+    }
+    info->multicast_table = str_list;
+    /* enable event notification after query */
+    nc->rxfilter_notify_enabled = true;
+
+    return info;
+}
+
 static void virtio_net_reset(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -420,6 +500,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
 {
     uint8_t on;
     size_t s;
+    NetClientState *nc = qemu_get_queue(n->nic);
 
     s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
     if (s != sizeof(on)) {
@@ -442,6 +523,8 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
         return VIRTIO_NET_ERR;
     }
 
+    rxfilter_notify(nc);
+
     return VIRTIO_NET_OK;
 }
 
@@ -487,6 +570,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 {
     struct virtio_net_ctrl_mac mac_data;
     size_t s;
+    NetClientState *nc = qemu_get_queue(n->nic);
 
     if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
         if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
@@ -495,6 +579,8 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
         s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
         assert(s == sizeof(n->mac));
         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
+        rxfilter_notify(nc);
+
         return VIRTIO_NET_OK;
     }
 
@@ -559,6 +645,8 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
         n->mac_table.multi_overflow = 1;
     }
 
+    rxfilter_notify(nc);
+
     return VIRTIO_NET_OK;
 }
 
@@ -1312,6 +1400,7 @@ static NetClientInfo net_virtio_info = {
     .receive = virtio_net_receive,
         .cleanup = virtio_net_cleanup,
     .link_status_changed = virtio_net_set_link_status,
+    .query_rx_filter = virtio_net_query_rxfilter,
 };
 
 static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1a6cfcf..1942cc4 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -41,6 +41,7 @@ typedef enum MonitorEvent {
     QEVENT_BLOCK_JOB_READY,
     QEVENT_DEVICE_DELETED,
     QEVENT_DEVICE_TRAY_MOVED,
+    QEVENT_NIC_RX_FILTER_CHANGED,
     QEVENT_SUSPEND,
     QEVENT_SUSPEND_DISK,
     QEVENT_WAKEUP,
diff --git a/include/net/net.h b/include/net/net.h
index 43d85a1..34d0d39 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -49,6 +49,7 @@ typedef ssize_t (NetReceiveIOV)(NetClientState *, const struct iovec *, int);
 typedef void (NetCleanup) (NetClientState *);
 typedef void (LinkStatusChanged)(NetClientState *);
 typedef void (NetClientDestructor)(NetClientState *);
+typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
 
 typedef struct NetClientInfo {
     NetClientOptionsKind type;
@@ -59,6 +60,7 @@ typedef struct NetClientInfo {
     NetCanReceive *can_receive;
     NetCleanup *cleanup;
     LinkStatusChanged *link_status_changed;
+    QueryRxFilter *query_rx_filter;
     NetPoll *poll;
 } NetClientInfo;
 
@@ -74,6 +76,7 @@ struct NetClientState {
     unsigned receive_disabled : 1;
     NetClientDestructor *destructor;
     unsigned int queue_index;
+    bool rxfilter_notify_enabled:true;
 };
 
 typedef struct NICState {
diff --git a/monitor.c b/monitor.c
index 6ce2a4e..75a66b6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -490,6 +490,7 @@ static const char *monitor_event_names[] = {
     [QEVENT_BLOCK_JOB_READY] = "BLOCK_JOB_READY",
     [QEVENT_DEVICE_DELETED] = "DEVICE_DELETED",
     [QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
+    [QEVENT_NIC_RX_FILTER_CHANGED] = "NIC_RX_FILTER_CHANGED",
     [QEVENT_SUSPEND] = "SUSPEND",
     [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
     [QEVENT_WAKEUP] = "WAKEUP",
@@ -2763,6 +2764,14 @@ static mon_cmd_t info_cmds[] = {
         .mhandler.cmd = hmp_info_tpm,
     },
     {
+        .name       = "rx-filter",
+        .args_type  = "name:s?",
+        .params     = "[net client name]",
+        .help       = "show the rx-filter information for all nics (or"
+                      " for the given nic)",
+        .mhandler.cmd = hmp_info_rx_filter,
+    },
+    {
         .name       = NULL,
     },
 };
diff --git a/net/net.c b/net/net.c
index 43a74e4..33abffe 100644
--- a/net/net.c
+++ b/net/net.c
@@ -961,6 +961,53 @@ void print_net_client(Monitor *mon, NetClientState *nc)
                    nc->info_str);
 }
 
+RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
+                                      Error **errp)
+{
+    NetClientState *nc;
+    RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
+
+    QTAILQ_FOREACH(nc, &net_clients, next) {
+        RxFilterInfoList *entry;
+        RxFilterInfo *info;
+
+        /* only query rx-filter information of nic */
+        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
+            continue;
+        }
+        if (has_name && strcmp(nc->name, name) != 0) {
+            continue;
+        }
+
+        if (nc->info->query_rx_filter) {
+            info = nc->info->query_rx_filter(nc);
+            entry = g_malloc0(sizeof(*entry));
+            entry->value = info;
+
+            if (!filter_list) {
+                filter_list = entry;
+            } else {
+                last_entry->next = entry;
+            }
+            last_entry = entry;
+        } else if (has_name) {
+            error_setg(errp, "net client(%s) doesn't support"
+                       " rx-filter querying", name);
+            break;
+        }
+    }
+
+    if (filter_list == NULL && !error_is_set(errp)) {
+        if (has_name) {
+            error_setg(errp, "invalid net client name: %s", name);
+        } else {
+            error_setg(errp, "no net client supports rx-filter querying");
+        }
+    }
+
+    return filter_list;
+}
+
 void do_info_network(Monitor *mon, const QDict *qdict)
 {
     NetClientState *nc, *peer;
diff --git a/qapi-schema.json b/qapi-schema.json
index 664b31f..0962809 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3619,3 +3619,75 @@
             '*cpuid-input-ecx': 'int',
             'cpuid-register': 'X86CPURegister32',
             'features': 'int' } }
+
+##
+# @RxState:
+#
+# Packets receiving state
+#
+# @normal: filter assigned packets according to the mac-table
+#
+# @none: don't receive any assigned packet
+#
+# @all: receive all assigned packets
+#
+##
+{ 'enum': 'RxState', 'data': [ 'normal', 'none', 'all' ] }
+
+##
+# @RxFilterInfo:
+#
+# Rx-filter information for a nic.
+#
+# @name: net client name
+#
+# @promiscuous: whether promiscuous mode is enabled
+#
+# @multicast: multicast receive state
+#
+# @unicast: unicast receive state
+#
+# @broadcast-allowed: whether to receive broadcast
+#
+# @multicast-overflow: multicast table is overflowed or not
+#
+# @unicast-overflow: unicast table is overflowed or not
+#
+# @main-mac: the main macaddr string
+#
+# @unicast-table: a list of unicast macaddr string
+#
+# @multicast-table: a list of multicast macaddr string
+#
+# Since 1.6
+##
+
+{ 'type': 'RxFilterInfo',
+  'data': {
+    'name':               'str',
+    'promiscuous':        'bool',
+    'multicast':          'RxState',
+    'unicast':            'RxState',
+    'broadcast-allowed':  'bool',
+    'multicast-overflow': 'bool',
+    'unicast-overflow':   'bool',
+    'main-mac':           'str',
+    'unicast-table':      ['str'],
+    'multicast-table':    ['str'] }}
+
+##
+# @query-rx-filter:
+#
+# Return rx-filter information for all nics (or for the given nic).
+#
+# @name: #optional net client name
+#
+# Returns: list of @RxFilterInfo for all nics (or for the given nic).
+#          Returns an error if the given @name doesn't exist, or given
+#          nic doesn't support rx-filter querying, or no net client
+#          supports rx-filter querying
+#
+# Since: 1.6
+##
+{ 'command': 'query-rx-filter', 'data': { '*name': 'str' },
+  'returns': ['RxFilterInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ffd130e..624ae5a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2932,3 +2932,61 @@ Example:
 <- { "return": {} }
 
 EQMP
+    {
+        .name       = "query-rx-filter",
+        .args_type  = "name:s?",
+        .mhandler.cmd_new = qmp_marshal_input_query_rx_filter,
+    },
+
+SQMP
+query-rx-filter
+---------------
+
+Show rx-filter information.
+
+Returns a json-array of rx-filter information for all nics (or for the
+given nic), returning an error if the given nic doesn't exist, or
+given nic doesn't support rx-filter querying, or no net client
+supports rx-filter querying.
+
+The query will clear the event notification flag of each nic, then qemu
+will start to emit event to QMP monitor.
+
+Each array entry contains the following:
+
+- "name": net client name (json-string)
+- "promiscuous": promiscuous mode is enabled (json-bool)
+- "multicast": multicast receive state (one of 'normal', 'none', 'all')
+- "unicast": unicast receive state  (one of 'normal', 'none', 'all')
+- "broadcast-allowed": allow to receive broadcast (json-bool)
+- "multicast-overflow": multicast table is overflowed (json-bool)
+- "unicast-overflow": unicast table is overflowed (json-bool)
+- "main-mac": main macaddr string (json-string)
+- "unicast-table": a json-array of unicast macaddr string
+- "multicast-table": a json-array of multicast macaddr string
+
+Example:
+
+-> { "execute": "query-rx-filter", "arguments": { "name": "vnet0" }}
+<- { "return": [
+        {
+            "promiscuous": true,
+            "name": "vnet0",
+            "main-mac": "52:54:00:12:34:56",
+            "unicast": "normal",
+            "unicast-table": [
+            ],
+            "multicast": "normal",
+            "multicast-overflow": false,
+            "unicast-overflow": false,
+            "multicast-table": [
+                "01:00:5e:00:00:01",
+                "33:33:00:00:00:01",
+                "33:33:ff:12:34:56"
+            ],
+            "broadcast-allowed": false
+        }
+      ]
+   }
+
+EQMP
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Michael S. Tsirkin]

On Fri, May 24, 2013 at 02:44:06PM +0800, Amos Kong wrote:
> We want to implement mac programming over macvtap through Libvirt. The
> related rx-filter information of the nic contains main mac, rx-mode
> items.
> 
> This patch adds a QMP event to notify management of rx-filter change,
> and adds a monitor command for management to query rx-filter
> information.
> 
> A flag for each nic is used to avoid events flooding, if management
> doesn't query rx-filter after it receives one event, new events won't
> be emitted to QMP monitor.
> 
> There maybe exist an uncontrollable delay, guests normally expect
> rx-filter updates immediately, but it's another separate issue, we
> can investigate it after Libvirt work is done.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> v2: add argument to filter mac-table info of single nic (Stefan)
>     update the document, add event notification
> v3: rename to rx-filter, add main mac, avoid events flooding (MST)
>     fix error process (Stefan), fix qmp interface (Eric)
> v4: process qerror in hmp, cleanup (Luiz)
>     set flag for each device, add device path in event, add
>     helper for g_strdup_printf (MST)
>     fix qmp document (Eric)
> ---
>  QMP/qmp-events.txt        | 19 ++++++++++
>  hmp-commands.hx           |  2 ++
>  hmp.c                     | 48 +++++++++++++++++++++++++
>  hmp.h                     |  1 +
>  hw/net/virtio-net.c       | 89 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/monitor/monitor.h |  1 +
>  include/net/net.h         |  3 ++
>  monitor.c                 |  9 +++++
>  net/net.c                 | 47 +++++++++++++++++++++++++
>  qapi-schema.json          | 72 ++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx           | 58 ++++++++++++++++++++++++++++++
>  11 files changed, 349 insertions(+)
> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 92fe5fb..315bbf8 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -172,6 +172,25 @@ Data:
>    },
>    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>  
> +NIC_RX_FILTER_CHANGED
> +-----------------
> +
> +Emitted when rx-filter configuration of nic is changed by the guest.
> +Each nic has a flag to control event emit, the flag is set to false
> +when it emits one event of the nic, the flag is set to true when
> +management queries the rx-filter of the nic. This is used to avoid
> +events flooding.
> +
> +Data:
> +
> +- "name": net client name (json-string)
> +
> +{ "event": "NIC_RX_FILTER_CHANGED",
> +  "data": { "name": "vnet0",
> +            "path": "/machine/peripheral/vnet0/virtio-backend" },
> +  "timestamp": { "seconds": 1368697518, "microseconds": 326866 } }
> +}
> +
>  RESET
>  -----
>  
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9cea415..b7863eb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1639,6 +1639,8 @@ show qdev device model list
>  show roms
>  @item info tpm
>  show the TPM device
> +@item info rx-filter [net client name]
> +show the rx-filter information for all nics (or for the given nic)
>  @end table
>  ETEXI
>  
> diff --git a/hmp.c b/hmp.c
> index 4fb76ec..af04eb1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -653,6 +653,54 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>      qapi_free_TPMInfoList(info_list);
>  }
>  
> +static void strlist_printf(Monitor *mon, strList *entry)
> +{
> +    while (entry) {
> +        monitor_printf(mon, "    %s\n", entry->value);
> +        entry = entry->next;
> +    }
> +}
> +
> +void hmp_info_rx_filter(Monitor *mon, const QDict *qdict)
> +{
> +    RxFilterInfoList *filter_list, *entry;
> +    bool has_name = qdict_haskey(qdict, "name");
> +    const char *name = NULL;
> +    Error *errp = NULL;
> +
> +    if (has_name) {
> +        name = qdict_get_str(qdict, "name");
> +    }
> +
> +    filter_list = entry = qmp_query_rx_filter(has_name, name, &errp);
> +    hmp_handle_error(mon, &errp);
> +
> +    while (entry) {
> +        monitor_printf(mon, "%s:\n", entry->value->name);
> +        monitor_printf(mon, " \\ promiscuous: %s\n",
> +                       entry->value->promiscuous ? "on" : "off");
> +        monitor_printf(mon, " \\ multicast: %s\n",
> +                       RxState_lookup[entry->value->multicast]);
> +        monitor_printf(mon, " \\ unicast: %s\n",
> +                       RxState_lookup[entry->value->unicast]);
> +        monitor_printf(mon, " \\ broadcast-allowed: %s\n",
> +                       entry->value->broadcast_allowed ? "on" : "off");
> +        monitor_printf(mon, " \\ multicast-overflow: %s\n",
> +                       entry->value->multicast_overflow ? "on" : "off");
> +        monitor_printf(mon, " \\ unicast-overflow: %s\n",
> +                       entry->value->unicast_overflow ? "on" : "off");
> +        monitor_printf(mon, " \\ main-mac: %s\n", entry->value->main_mac);
> +
> +        monitor_printf(mon, " \\ unicast-table:\n");
> +        strlist_printf(mon, entry->value->unicast_table);
> +        monitor_printf(mon, " \\ multicast-table:\n");
> +        strlist_printf(mon, entry->value->multicast_table);
> +
> +        entry = entry->next;
> +    }
> +    qapi_free_RxFilterInfoList(filter_list);
> +}
> +
>  void hmp_quit(Monitor *mon, const QDict *qdict)
>  {
>      monitor_suspend(mon);
> diff --git a/hmp.h b/hmp.h
> index 95fe76e..9af733e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_info_pci(Monitor *mon, const QDict *qdict);
>  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> +void hmp_info_rx_filter(Monitor *mon, const QDict *qdict);
>  void hmp_quit(Monitor *mon, const QDict *qdict);
>  void hmp_stop(Monitor *mon, const QDict *qdict);
>  void hmp_system_reset(Monitor *mon, const QDict *qdict);
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 1ea9556..56df052 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -21,6 +21,8 @@
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
>  #include "hw/virtio/virtio-bus.h"
> +#include "qapi/qmp/qjson.h"
> +#include "monitor/monitor.h"
>  
>  #define VIRTIO_NET_VM_VERSION    11
>  
> @@ -192,6 +194,84 @@ static void virtio_net_set_link_status(NetClientState *nc)
>      virtio_net_set_status(vdev, vdev->status);
>  }
>  
> +static void rxfilter_notify(NetClientState *nc)
> +{
> +    QObject *event_data;
> +    VirtIONet *n = qemu_get_nic_opaque(nc);
> +
> +    if (nc->rxfilter_notify_enabled) {
> +        event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
> +                           n->netclient_name,
> +                           object_get_canonical_path(OBJECT(n->qdev)));
> +        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
> +        qobject_decref(event_data);
> +        /* disable event notification to avoid events flooding */
> +        nc->rxfilter_notify_enabled = false;
> +    }
> +}
> +
> +static char *mac_strdup_printf(uint8_t *mac)
> +{
> +    return g_strdup_printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x", mac[0],
> +                            mac[1], mac[2], mac[3], mac[4], mac[5]);
> +}
> +
> +static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
> +{
> +    VirtIONet *n = qemu_get_nic_opaque(nc);
> +    RxFilterInfo *info;
> +    strList *str_list = NULL;
> +    strList *entry;
> +    int i;
> +
> +    info = g_malloc0(sizeof(*info));
> +    info->name = g_strdup(nc->name);
> +    info->promiscuous = n->promisc;
> +
> +    if (n->nouni) {
> +        info->unicast = RX_STATE_NONE;
> +    } else if (n->alluni) {
> +        info->unicast = RX_STATE_ALL;
> +    } else {
> +        info->unicast = RX_STATE_NORMAL;
> +    }
> +
> +    if (n->nomulti) {
> +        info->multicast = RX_STATE_NONE;
> +    } else if (n->allmulti) {
> +        info->multicast = RX_STATE_ALL;
> +    } else {
> +        info->multicast = RX_STATE_NORMAL;
> +    }
> +
> +    info->broadcast_allowed = n->nobcast;
> +    info->multicast_overflow = n->mac_table.multi_overflow;
> +    info->unicast_overflow = n->mac_table.uni_overflow;
> +
> +    info->main_mac = mac_strdup_printf(n->mac);
> +
> +    for (i = 0; i < n->mac_table.first_multi; i++) {
> +        entry = g_malloc0(sizeof(*entry));
> +        entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
> +        entry->next = str_list;
> +        str_list = entry;
> +    }
> +    info->unicast_table = str_list;
> +
> +    str_list = NULL;
> +    for (i = n->mac_table.first_multi; i < n->mac_table.in_use; i++) {
> +        entry = g_malloc0(sizeof(*entry));
> +        entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
> +        entry->next = str_list;
> +        str_list = entry;
> +    }
> +    info->multicast_table = str_list;
> +    /* enable event notification after query */
> +    nc->rxfilter_notify_enabled = true;
> +
> +    return info;
> +}
> +
>  static void virtio_net_reset(VirtIODevice *vdev)
>  {
>      VirtIONet *n = VIRTIO_NET(vdev);
> @@ -420,6 +500,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>  {
>      uint8_t on;
>      size_t s;
> +    NetClientState *nc = qemu_get_queue(n->nic);
>  
>      s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
>      if (s != sizeof(on)) {
> @@ -442,6 +523,8 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>          return VIRTIO_NET_ERR;
>      }
>  
> +    rxfilter_notify(nc);
> +
>      return VIRTIO_NET_OK;
>  }
>  
> @@ -487,6 +570,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>  {
>      struct virtio_net_ctrl_mac mac_data;
>      size_t s;
> +    NetClientState *nc = qemu_get_queue(n->nic);
>  
>      if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
>          if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
> @@ -495,6 +579,8 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>          s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
>          assert(s == sizeof(n->mac));
>          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> +        rxfilter_notify(nc);
> +
>          return VIRTIO_NET_OK;
>      }
>  
> @@ -559,6 +645,8 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>          n->mac_table.multi_overflow = 1;
>      }
>  
> +    rxfilter_notify(nc);
> +
>      return VIRTIO_NET_OK;
>  }
>  
> @@ -1312,6 +1400,7 @@ static NetClientInfo net_virtio_info = {
>      .receive = virtio_net_receive,
>          .cleanup = virtio_net_cleanup,
>      .link_status_changed = virtio_net_set_link_status,
> +    .query_rx_filter = virtio_net_query_rxfilter,
>  };
>  
>  static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 1a6cfcf..1942cc4 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>      QEVENT_BLOCK_JOB_READY,
>      QEVENT_DEVICE_DELETED,
>      QEVENT_DEVICE_TRAY_MOVED,
> +    QEVENT_NIC_RX_FILTER_CHANGED,
>      QEVENT_SUSPEND,
>      QEVENT_SUSPEND_DISK,
>      QEVENT_WAKEUP,
> diff --git a/include/net/net.h b/include/net/net.h
> index 43d85a1..34d0d39 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -49,6 +49,7 @@ typedef ssize_t (NetReceiveIOV)(NetClientState *, const struct iovec *, int);
>  typedef void (NetCleanup) (NetClientState *);
>  typedef void (LinkStatusChanged)(NetClientState *);
>  typedef void (NetClientDestructor)(NetClientState *);
> +typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
>  
>  typedef struct NetClientInfo {
>      NetClientOptionsKind type;
> @@ -59,6 +60,7 @@ typedef struct NetClientInfo {
>      NetCanReceive *can_receive;
>      NetCleanup *cleanup;
>      LinkStatusChanged *link_status_changed;
> +    QueryRxFilter *query_rx_filter;
>      NetPoll *poll;
>  } NetClientInfo;
>  
> @@ -74,6 +76,7 @@ struct NetClientState {
>      unsigned receive_disabled : 1;
>      NetClientDestructor *destructor;
>      unsigned int queue_index;
> +    bool rxfilter_notify_enabled:true;
>  };
>  
>  typedef struct NICState {
> diff --git a/monitor.c b/monitor.c
> index 6ce2a4e..75a66b6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -490,6 +490,7 @@ static const char *monitor_event_names[] = {
>      [QEVENT_BLOCK_JOB_READY] = "BLOCK_JOB_READY",
>      [QEVENT_DEVICE_DELETED] = "DEVICE_DELETED",
>      [QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
> +    [QEVENT_NIC_RX_FILTER_CHANGED] = "NIC_RX_FILTER_CHANGED",
>      [QEVENT_SUSPEND] = "SUSPEND",
>      [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
>      [QEVENT_WAKEUP] = "WAKEUP",
> @@ -2763,6 +2764,14 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.cmd = hmp_info_tpm,
>      },
>      {
> +        .name       = "rx-filter",
> +        .args_type  = "name:s?",
> +        .params     = "[net client name]",
> +        .help       = "show the rx-filter information for all nics (or"
> +                      " for the given nic)",
> +        .mhandler.cmd = hmp_info_rx_filter,
> +    },
> +    {
>          .name       = NULL,
>      },
>  };
> diff --git a/net/net.c b/net/net.c
> index 43a74e4..33abffe 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -961,6 +961,53 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>                     nc->info_str);
>  }
>  
> +RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
> +                                      Error **errp)
> +{
> +    NetClientState *nc;
> +    RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
> +
> +    QTAILQ_FOREACH(nc, &net_clients, next) {
> +        RxFilterInfoList *entry;
> +        RxFilterInfo *info;
> +
> +        /* only query rx-filter information of nic */
> +        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
> +            continue;
> +        }
> +        if (has_name && strcmp(nc->name, name) != 0) {
> +            continue;
> +        }
> +
> +        if (nc->info->query_rx_filter) {
> +            info = nc->info->query_rx_filter(nc);
> +            entry = g_malloc0(sizeof(*entry));
> +            entry->value = info;
> +
> +            if (!filter_list) {
> +                filter_list = entry;
> +            } else {
> +                last_entry->next = entry;
> +            }
> +            last_entry = entry;
> +        } else if (has_name) {
> +            error_setg(errp, "net client(%s) doesn't support"
> +                       " rx-filter querying", name);
> +            break;
> +        }
> +    }
> +
> +    if (filter_list == NULL && !error_is_set(errp)) {
> +        if (has_name) {
> +            error_setg(errp, "invalid net client name: %s", name);
> +        } else {
> +            error_setg(errp, "no net client supports rx-filter querying");
> +        }
> +    }
> +
> +    return filter_list;
> +}
> +
>  void do_info_network(Monitor *mon, const QDict *qdict)
>  {
>      NetClientState *nc, *peer;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 664b31f..0962809 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3619,3 +3619,75 @@
>              '*cpuid-input-ecx': 'int',
>              'cpuid-register': 'X86CPURegister32',
>              'features': 'int' } }
> +
> +##
> +# @RxState:
> +#
> +# Packets receiving state
> +#
> +# @normal: filter assigned packets according to the mac-table
> +#
> +# @none: don't receive any assigned packet
> +#
> +# @all: receive all assigned packets
> +#
> +##
> +{ 'enum': 'RxState', 'data': [ 'normal', 'none', 'all' ] }
> +
> +##
> +# @RxFilterInfo:
> +#
> +# Rx-filter information for a nic.
> +#
> +# @name: net client name
> +#
> +# @promiscuous: whether promiscuous mode is enabled
> +#
> +# @multicast: multicast receive state
> +#
> +# @unicast: unicast receive state
> +#
> +# @broadcast-allowed: whether to receive broadcast
> +#
> +# @multicast-overflow: multicast table is overflowed or not
> +#
> +# @unicast-overflow: unicast table is overflowed or not
> +#
> +# @main-mac: the main macaddr string
> +#
> +# @unicast-table: a list of unicast macaddr string
> +#
> +# @multicast-table: a list of multicast macaddr string
> +#
> +# Since 1.6
> +##
> +
> +{ 'type': 'RxFilterInfo',
> +  'data': {
> +    'name':               'str',
> +    'promiscuous':        'bool',
> +    'multicast':          'RxState',
> +    'unicast':            'RxState',
> +    'broadcast-allowed':  'bool',
> +    'multicast-overflow': 'bool',
> +    'unicast-overflow':   'bool',
> +    'main-mac':           'str',
> +    'unicast-table':      ['str'],
> +    'multicast-table':    ['str'] }}
> +
> +##
> +# @query-rx-filter:
> +#
> +# Return rx-filter information for all nics (or for the given nic).
> +#
> +# @name: #optional net client name
> +#
> +# Returns: list of @RxFilterInfo for all nics (or for the given nic).
> +#          Returns an error if the given @name doesn't exist, or given
> +#          nic doesn't support rx-filter querying, or no net client
> +#          supports rx-filter querying
> +#
> +# Since: 1.6
> +##
> +{ 'command': 'query-rx-filter', 'data': { '*name': 'str' },
> +  'returns': ['RxFilterInfo'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ffd130e..624ae5a 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2932,3 +2932,61 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +    {
> +        .name       = "query-rx-filter",
> +        .args_type  = "name:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_query_rx_filter,
> +    },
> +
> +SQMP
> +query-rx-filter
> +---------------
> +
> +Show rx-filter information.
> +
> +Returns a json-array of rx-filter information for all nics (or for the
> +given nic), returning an error if the given nic doesn't exist, or
> +given nic doesn't support rx-filter querying, or no net client
> +supports rx-filter querying.
> +
> +The query will clear the event notification flag of each nic, then qemu
> +will start to emit event to QMP monitor.
> +
> +Each array entry contains the following:
> +
> +- "name": net client name (json-string)
> +- "promiscuous": promiscuous mode is enabled (json-bool)
> +- "multicast": multicast receive state (one of 'normal', 'none', 'all')
> +- "unicast": unicast receive state  (one of 'normal', 'none', 'all')
> +- "broadcast-allowed": allow to receive broadcast (json-bool)
> +- "multicast-overflow": multicast table is overflowed (json-bool)
> +- "unicast-overflow": unicast table is overflowed (json-bool)
> +- "main-mac": main macaddr string (json-string)
> +- "unicast-table": a json-array of unicast macaddr string
> +- "multicast-table": a json-array of multicast macaddr string

How are these sorted by the way?

> +
> +Example:
> +
> +-> { "execute": "query-rx-filter", "arguments": { "name": "vnet0" }}
> +<- { "return": [
> +        {
> +            "promiscuous": true,
> +            "name": "vnet0",
> +            "main-mac": "52:54:00:12:34:56",
> +            "unicast": "normal",
> +            "unicast-table": [
> +            ],
> +            "multicast": "normal",
> +            "multicast-overflow": false,
> +            "unicast-overflow": false,
> +            "multicast-table": [
> +                "01:00:5e:00:00:01",
> +                "33:33:00:00:00:01",
> +                "33:33:ff:12:34:56"
> +            ],
> +            "broadcast-allowed": false
> +        }
> +      ]
> +   }
> +
> +EQMP
> -- 
> 1.8.1.4

Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Eric Blake]

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

On 05/24/2013 06:03 AM, Michael S. Tsirkin wrote:
> On Fri, May 24, 2013 at 02:44:06PM +0800, Amos Kong wrote:
>> We want to implement mac programming over macvtap through Libvirt. The
>> related rx-filter information of the nic contains main mac, rx-mode
>> items.
>>

>> +Each array entry contains the following:
>> +
>> +- "name": net client name (json-string)
>> +- "promiscuous": promiscuous mode is enabled (json-bool)
>> +- "multicast": multicast receive state (one of 'normal', 'none', 'all')
>> +- "unicast": unicast receive state  (one of 'normal', 'none', 'all')
>> +- "broadcast-allowed": allow to receive broadcast (json-bool)
>> +- "multicast-overflow": multicast table is overflowed (json-bool)
>> +- "unicast-overflow": unicast table is overflowed (json-bool)
>> +- "main-mac": main macaddr string (json-string)
>> +- "unicast-table": a json-array of unicast macaddr string
>> +- "multicast-table": a json-array of multicast macaddr string
> 
> How are these sorted by the way?

They don't have to be - JSON uses name-value pairs in dictionaries
precisely because they aren't sorted.  However, it looks like you
matched the order that you listed in the qapi-schema.json file, which is
as good as any (even if it differs from the random hash ordering
demonstrated in your example below).  Or are you asking how macaddr
strings within multicast-table are sorted (JSON arrays DO convey
ordering relations), rather than how the name-value pairs are (not)
sorted in the overall array entry dictionary?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Eric Blake]

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

On 05/24/2013 12:44 AM, Amos Kong wrote:
> We want to implement mac programming over macvtap through Libvirt. The
> related rx-filter information of the nic contains main mac, rx-mode
> items.
> 
> This patch adds a QMP event to notify management of rx-filter change,
> and adds a monitor command for management to query rx-filter
> information.
> 
> A flag for each nic is used to avoid events flooding, if management
> doesn't query rx-filter after it receives one event, new events won't
> be emitted to QMP monitor.
> 
> There maybe exist an uncontrollable delay, guests normally expect
> rx-filter updates immediately, but it's another separate issue, we
> can investigate it after Libvirt work is done.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---

> +++ b/QMP/qmp-events.txt
> @@ -172,6 +172,25 @@ Data:
>    },
>    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>  
> +NIC_RX_FILTER_CHANGED
> +-----------------
> +
> +Emitted when rx-filter configuration of nic is changed by the guest.
> +Each nic has a flag to control event emit, the flag is set to false
> +when it emits one event of the nic, the flag is set to true when
> +management queries the rx-filter of the nic. This is used to avoid
> +events flooding.
> +
> +Data:
> +
> +- "name": net client name (json-string)

Missing documentation on the "path" member.

> +
> +{ "event": "NIC_RX_FILTER_CHANGED",
> +  "data": { "name": "vnet0",
> +            "path": "/machine/peripheral/vnet0/virtio-backend" },
> +  "timestamp": { "seconds": 1368697518, "microseconds": 326866 } }
> +}

Hmm - if we are going to add introspection of events, we probably need
to improve this file to call out ALL the events in JSON format (not just
English text describing the event and then an example of an emitted
event) - but that's a different task for a different series.

> +++ b/include/net/net.h
> @@ -74,6 +76,7 @@ struct NetClientState {
>      unsigned receive_disabled : 1;
>      NetClientDestructor *destructor;
>      unsigned int queue_index;
> +    bool rxfilter_notify_enabled:true;

Very unusual to use 'true' instead of '1' for a bitfield length (valid
C, but I've never seen it done in the wild).  Also, the code base has
very few bit fields of type bool (I only found 4: two in
block/raw-posix.c and two in hw/intc/openpic.c); we generally prefer
'int' or 'unsigned' bitfields, where bitfields make sense.  For that
matter, is this really a struct where we are trying to cram as much
information into space such that a bitfield helps us, or can we just use
plain 'bool' instead of trying to make it a bitfield?  And if a bitfield
is important, then why not group this next to the receive_disabled
bitfield so that the two share the same byte, instead of wasting
alignment for two disjoint bitfields?

Everything else looked okay to me.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Amos Kong]

On Fri, May 24, 2013 at 06:26:40AM -0600, Eric Blake wrote:
> On 05/24/2013 06:03 AM, Michael S. Tsirkin wrote:
> > On Fri, May 24, 2013 at 02:44:06PM +0800, Amos Kong wrote:
> >> We want to implement mac programming over macvtap through Libvirt. The
> >> related rx-filter information of the nic contains main mac, rx-mode
> >> items.
> >>
> 
> >> +Each array entry contains the following:
> >> +
> >> +- "name": net client name (json-string)
> >> +- "promiscuous": promiscuous mode is enabled (json-bool)
> >> +- "multicast": multicast receive state (one of 'normal', 'none', 'all')
> >> +- "unicast": unicast receive state  (one of 'normal', 'none', 'all')
> >> +- "broadcast-allowed": allow to receive broadcast (json-bool)
> >> +- "multicast-overflow": multicast table is overflowed (json-bool)
> >> +- "unicast-overflow": unicast table is overflowed (json-bool)
> >> +- "main-mac": main macaddr string (json-string)
> >> +- "unicast-table": a json-array of unicast macaddr string
> >> +- "multicast-table": a json-array of multicast macaddr string
> > 
> > How are these sorted by the way?
 
The order is same as in 'struct VirtIONet'. I just keep this order match
the order in qapi-schema.json.

> They don't have to be - JSON uses name-value pairs in dictionaries
> precisely because they aren't sorted.  However, it looks like you
> matched the order that you listed in the qapi-schema.json file, which is
> as good as any (even if it differs from the random hash ordering
> demonstrated in your example below).

Yes.

> Or are you asking how macaddr
> strings within multicast-table are sorted (JSON arrays DO convey
> ordering relations),

macaddr strings order in QMP output is decided by python dictionary.
macaddr strings order in HMP output is same as the order in guest
mac-table.

> rather than how the name-value pairs are (not)
> sorted in the overall array entry dictionary?

-- 
			Amos.

Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information

[Amos Kong]

On Fri, May 24, 2013 at 06:42:34AM -0600, Eric Blake wrote:
> On 05/24/2013 12:44 AM, Amos Kong wrote:
> > We want to implement mac programming over macvtap through Libvirt. The
> > related rx-filter information of the nic contains main mac, rx-mode
> > items.
> > 
> > This patch adds a QMP event to notify management of rx-filter change,
> > and adds a monitor command for management to query rx-filter
> > information.
> > 
> > A flag for each nic is used to avoid events flooding, if management
> > doesn't query rx-filter after it receives one event, new events won't
> > be emitted to QMP monitor.
> > 
> > There maybe exist an uncontrollable delay, guests normally expect
> > rx-filter updates immediately, but it's another separate issue, we
> > can investigate it after Libvirt work is done.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> 
> > +++ b/QMP/qmp-events.txt
> > @@ -172,6 +172,25 @@ Data:
> >    },
> >    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> >  
> > +NIC_RX_FILTER_CHANGED
> > +-----------------
> > +
> > +Emitted when rx-filter configuration of nic is changed by the guest.
> > +Each nic has a flag to control event emit, the flag is set to false
> > +when it emits one event of the nic, the flag is set to true when
> > +management queries the rx-filter of the nic. This is used to avoid
> > +events flooding.
> > +
> > +Data:
> > +
> > +- "name": net client name (json-string)
> 
> Missing documentation on the "path" member.
 
fixed.

> > +++ b/include/net/net.h
> > @@ -74,6 +76,7 @@ struct NetClientState {
> >      unsigned receive_disabled : 1;
> >      NetClientDestructor *destructor;
> >      unsigned int queue_index;
> > +    bool rxfilter_notify_enabled:true;
> 
> Very unusual to use 'true' instead of '1' for a bitfield length (valid
> C, but I've never seen it done in the wild).  Also, the code base has
> very few bit fields of type bool (I only found 4: two in
> block/raw-posix.c and two in hw/intc/openpic.c);

> we generally prefer
> 'int' or 'unsigned' bitfields, where bitfields make sense.  For that
> matter, is this really a struct where we are trying to cram as much
> information into space such that a bitfield helps us, or can we just use
> plain 'bool' instead of trying to make it a bitfield?

> And if a bitfield
> is important, then why not group this next to the receive_disabled
> bitfield so that the two share the same byte, instead of wasting
> alignment for two disjoint bitfields?

it's more complex.

receive_disabled was used as a 'bool' not bitfield here. I would like
to add a new variable to indicate if notification is needed.

         unsigned rxfilter_notify_enabled:1;
 
> Everything else looked okay to me.



-- 
			Amos.