On Thu, Jul 11, 2013 at 10:28:15PM +0800, Amos Kong wrote:
> 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 <ak...@redhat.com>
I applied this and folded into your previous
patch. Pushed on my pci branch.
Amos, could you please verify (and test)
that the result is correct?
Thanks,
> ---
> 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
