On 05/23/2013 03:08 AM, Amos Kong wrote: > We want to implement mac programming over macvtap through Libvirt, > related rx-filter configuration contains main mac, some of rx-mode > and mac-table. > > The previous patch adds QMP event to notify management of rx-filter > change. This patch adds a monitor command to query rx-filter > information. > > A flag is used to avoid events flooding, if user don't query
s/don't/doesn't/
> rx-filter after receives one event, new events won't be sent
s/after/after it/
> to qmp monitor.
>
> +++ 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;
> +
> + if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
> + continue;
> + }
> + if (has_name && strcmp(nc->name, name) != 0) {
Do you need the has_name argument here, or can you ensure that the
caller passes NULL when the caller's has_name was false, for one less
parameter and the same amount of information?
> +++ b/qapi-schema.json
> @@ -3619,3 +3619,76 @@
> '*cpuid-input-ecx': 'int',
> 'cpuid-register': 'X86CPURegister32',
> 'features': 'int' } }
> +
> +##
> +# @RxState:
> +#
> +# Packets receiving state
> +#
> +# @normal: filter assigned packets according to the mac-table
> +#
> +# @no: don't receive any assigned packet
> +#
> +# @all: receive all assigned packets
> +#
> +##
> +{ 'enum': 'RxState', 'data': [ 'normal', 'no', 'all' ] }
I think s/no/none/ would make slightly more sense (usually, you pair
no/yes and none/all, not no/all).
> +
> +##
> +# @RxFilterInfo:
> +#
> +# Rx-filter information for a net client, it contains main mac, some
> +# of rx-mode items and mac-table.
> +#
> +# @name: net client name
> +#
> +# @promiscuous: whether to ether promiscuous mode
s/to ether//; s/$/is enabled/
> +#
> +# @multicast: multicast receive state
> +#
> +# @unicast: unicast receive state
> +#
> +# @broadcast-allowed: whether to receive broadcast
> +#
> +# @multicast-overflow: multicast table is overflow or not
> +#
> +# @unicast-overflow: unicast table is overflow or not
> +#
> +# @main-mac: the main macaddr string
> +#
> +# @unicast-table: a list of unicast macaddr string
> +#
> +# @multicast-table: a list of multicast macaddr string
Naming is reasonable; thanks for improving things from v1.
> +#
> +# 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'] }
Interface looks reasonable. I didn't check the code closely, but agree
with Michael's assessment that the event-suppression flag has to be
per-device, not global.
> +
> +Each array entry contains the following:
> +
> +- "name": net client name (jaso-string)
> +- "promiscuous": enter promiscuous mode (json-bool)
> +- "multicast": multicast receive state (one of 'normal', 'no', 'all')
> +- "unicast": unicast receive state (one of 'normal', 'no', 'all')
> +- "broadcast-allowed": allow to receive broadcast (json-bool)
> +- "multicast-overflow": multicast table is overflow (json-bool)
s/is overflow/overflowed/
> +- "unicast-overflow": unicast table is overflow (json-bool)
and again
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
