[Qemu-devel] [PATCH MST/PCI] additional fixes for mac-programming feature

[Amos Kong]

Markus added some comments on old patchset, this patch contains
some additional fixes, it's based on MST's PCI tree.

* Fix typos (missed 1.6, NIC)
* Don't initialize list point at its declaration
* Always notify QMP client if mactable is changed
* Returns NULL list if no net client supports rx-filter querying.

BTW, we can also use e1000 with macvtap device, so we can implement
a query_rx_filter function for e1000 in future.

Signed-off-by: Amos Kong <address@hidden>
---
 hw/net/virtio-net.c | 24 ++++++++++++++----------
 net/net.c           | 19 ++++++++++---------
 qapi-schema.json    | 11 ++++++-----
 qmp-commands.hx     | 10 +++++-----
 4 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c88403a..679f50c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -226,10 +226,8 @@ static RxFilterInfo 
*virtio_net_query_rxfilter(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     RxFilterInfo *info;
-    strList *str_list = NULL;
-    strList *entry;
-    intList *int_list = NULL;
-    intList *int_entry;
+    strList *str_list, *entry;
+    intList *int_list, *int_entry;
     int i, j;
 
     info = g_malloc0(sizeof(*info));
@@ -258,6 +256,7 @@ static RxFilterInfo 
*virtio_net_query_rxfilter(NetClientState *nc)
 
     info->main_mac = mac_strdup_printf(n->mac);
 
+    str_list = NULL;
     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);
@@ -275,6 +274,7 @@ static RxFilterInfo 
*virtio_net_query_rxfilter(NetClientState *nc)
     }
     info->multicast_table = str_list;
 
+    int_list = NULL;
     for (i = 0; i < MAX_VLAN >> 5; i++) {
         for (j = 0; n->vlans[i] && j < 0x1f; j++) {
             if (n->vlans[i] & (1U << j)) {
@@ -619,19 +619,19 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
                    sizeof(mac_data.entries));
     mac_data.entries = ldl_p(&mac_data.entries);
     if (s != sizeof(mac_data.entries)) {
-        return VIRTIO_NET_ERR;
+        goto error;
     }
     iov_discard_front(&iov, &iov_cnt, s);
 
     if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
-        return VIRTIO_NET_ERR;
+        goto error;
     }
 
     if (mac_data.entries <= MAC_TABLE_ENTRIES) {
         s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
                        mac_data.entries * ETH_ALEN);
         if (s != mac_data.entries * ETH_ALEN) {
-            return VIRTIO_NET_ERR;
+            goto error;
         }
         n->mac_table.in_use += mac_data.entries;
     } else {
@@ -646,20 +646,20 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
                    sizeof(mac_data.entries));
     mac_data.entries = ldl_p(&mac_data.entries);
     if (s != sizeof(mac_data.entries)) {
-        return VIRTIO_NET_ERR;
+        goto error;
     }
 
     iov_discard_front(&iov, &iov_cnt, s);
 
     if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
-        return VIRTIO_NET_ERR;
+        goto error;
     }
 
     if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
         s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
                        mac_data.entries * ETH_ALEN);
         if (s != mac_data.entries * ETH_ALEN) {
-            return VIRTIO_NET_ERR;
+            goto error;
         }
         n->mac_table.in_use += mac_data.entries;
     } else {
@@ -669,6 +669,10 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
     rxfilter_notify(nc);
 
     return VIRTIO_NET_OK;
+
+error:
+    rxfilter_notify(nc);
+    return VIRTIO_NET_ERR;
 }
 
 static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
diff --git a/net/net.c b/net/net.c
index 33abffe..c0d61bf 100644
--- a/net/net.c
+++ b/net/net.c
@@ -971,11 +971,16 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, 
const char *name,
         RxFilterInfoList *entry;
         RxFilterInfo *info;
 
-        /* only query rx-filter information of nic */
-        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
+        if (has_name && strcmp(nc->name, name) != 0) {
             continue;
         }
-        if (has_name && strcmp(nc->name, name) != 0) {
+
+        /* only query rx-filter information of NIC */
+        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
+            if (has_name) {
+                error_setg(errp, "net client(%s) isn't a NIC", name);
+                break;
+            }
             continue;
         }
 
@@ -997,12 +1002,8 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, 
const char *name,
         }
     }
 
-    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");
-        }
+    if (filter_list == NULL && !error_is_set(errp) && has_name) {
+        error_setg(errp, "invalid net client name: %s", name);
     }
 
     return filter_list;
diff --git a/qapi-schema.json b/qapi-schema.json
index da45980..95ba5a6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3724,13 +3724,14 @@
 #
 # @all: receive all assigned packets
 #
+# Since: 1.6
 ##
 { 'enum': 'RxState', 'data': [ 'normal', 'none', 'all' ] }
 
 ##
 # @RxFilterInfo:
 #
-# Rx-filter information for a nic.
+# Rx-filter information for a NIC.
 #
 # @name: net client name
 #
@@ -3774,14 +3775,14 @@
 ##
 # @query-rx-filter:
 #
-# Return rx-filter information for all nics (or for the given nic).
+# 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: 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
+#          NIC doesn't support rx-filter querying, or given net client
+#          isn't a NIC.
 #
 # Since: 1.6
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 37113e5..aee3f96 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3055,12 +3055,12 @@ 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.
+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 given net client
+isn't a NIC.
 
-The query will clear the event notification flag of each nic, then qemu
+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:
-- 
1.8.3.1