Re: [PATCH v2 14/31] qapi/qom: Add ObjectOptions for filter-*

[Eric Blake]

On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the filter-* objects.
> 
> Some parts of the interface (in particular NetfilterProperties.position)
> are very unusual for QAPI, but for now just describe the existing
> interface.
> 
> net.json can't be included in qom.json because the storage daemon
> doesn't have it. NetFilterDirection is still required in the new object
> property definitions in qom.json, so move this enum to common.json.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/common.json |  20 +++++++
>  qapi/net.json    |  20 -------
>  qapi/qom.json    | 143 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 163 insertions(+), 20 deletions(-)
> 

> +++ b/qapi/qom.json
> @@ -313,6 +313,137 @@
>    'data': { 'addr': 'str' ,
>              '*id-list': 'str' } }
>  
> +##
> +# @NetfilterInsert:
> +#
> +# Indicates where to insert a netfilter relative to a given other filter.
> +#
> +# @before: insert before the specified filter
> +#
> +# @behind: insert behind the specified filter
> +#
> +# Since: 5.0
> +##
> +{ 'enum': 'NetfilterInsert',
> +  'data': [ 'before', 'behind' ] }
> +
> +##
> +# @NetfilterProperties:
> +#
> +# Properties for objects of classes derived from netfilter.
> +#
> +# @netdev: id of the network device backend to filter
> +#
> +# @queue: indicates which queue(s) to filter (default: all)
> +#
> +# @status: indicates whether the filter is enabled ("on") or disabled ("off")
> +#          (default: "on")

An enum would be nicer than 'str', but your commit message is accurate.

> +#
> +# @position: specifies where the filter should be inserted in the filter 
> list.
> +#            "head" means the filter is inserted at the head of the filter 
> list,
> +#            before any existing filters.
> +#            "tail" means the filter is inserted at the tail of the filter 
> list,
> +#            behind any existing filters (default).
> +#            "id=<id>" means the filter is inserted before or behind the 
> filter
> +#            specified by <id>, depending on the @insert property.
> +#            (default: "tail")
> +#

Wow, you're not kidding about this not being typical QAPI.  Oh well.

> +# @insert: where to insert the filter relative to the filter given in 
> @position.
> +#          Ignored if @position is "head" or "tail". (default: behind)

Back to the question of if it is worth updating the QAPI generator to
allow a flat union as the branch of yet another flat union.  If we did
that, we could have (untested):

{ 'enum': 'NetfilterPosition', 'data': [ 'head', 'tail', 'id' ] }
{ 'union': 'NetfilterBase',
  'base': { 'position': 'NetfilterPosition',
            'netdev'..., 'queue', 'status'... },
  'discriminator': 'position',
  'data': { 'head': {}, 'tail': {},
            'id': { '*insert': 'NetfilterInsert', 'id': 'str' } }

but that is a change to our existing id=xyz parsing, so we may need an
alias or deprecation period...

> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'NetfilterProperties',
> +  'data': { 'netdev': 'str',
> +            '*queue': 'NetFilterDirection',
> +            '*status': 'str',
> +            '*position': 'str',
> +            '*insert': 'NetfilterInsert' } }
> +
> +##
> +# @FilterBufferProperties:
> +#
> +# Properties for filter-buffer objects.
> +#
> +# @interval: a non-zero interval in microseconds.  All packets arriving in 
> the
> +#            given interval are delayed until the end of the interval.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'FilterBufferProperties',
> +  'base': 'NetfilterProperties',
> +  'data': { 'interval': 'uint32' } }

matches net/filter-buffer.c:filter_buffer_class_init().

> +
> +##
> +# @FilterDumpProperties:
> +#
> +# Properties for filter-dump objects.
> +#
> +# @file: the filename where the dumped packets should be stored
> +#
> +# @maxlen: maximum number of bytes in a packet that are stored (default: 
> 65536)
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'FilterDumpProperties',
> +  'base': 'NetfilterProperties',
> +  'data': { 'file': 'str',
> +            '*maxlen': 'uint32' } }

Matches net/dump.c:filter_dump_class_init().

> +
> +##
> +# @FilterMirrorProperties:
> +#
> +# Properties for filter-mirror objects.
> +#
> +# @outdev: the name of a character device backend to which all incoming 
> packets
> +#          are mirrored
> +#
> +# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'FilterMirrorProperties',
> +  'base': 'NetfilterProperties',
> +  'data': { 'outdev': 'str',
> +            '*vnet_hdr_support': 'bool' } }

Matches filter-mirror.c:filter_mirror_class_init().  For the future, can
we rename to vnet-hdr-support?

> +
> +##
> +# @FilterRedirectorProperties:
> +#
> +# Properties for filter-redirector objects.
> +#
> +# At least one of @indev or @outdev must be present.  If both are present, 
> they
> +# must not refer to the same character device backend.
> +#
> +# @indev: the name of a character device backend from which packets are
> +#         received and redirected to the filtered network device
> +#
> +# @outdev: the name of a character device backend to which all incoming 
> packets
> +#          are redirected
> +#
> +# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'FilterRedirectorProperties',
> +  'base': 'NetfilterProperties',
> +  'data': { '*indev': 'str',
> +            '*outdev': 'str',
> +            '*vnet_hdr_support': 'bool' } }

Matches net/filter-mirror.c:filter_redirector_class_init().

> +
> +##
> +# @FilterRewriterProperties:
> +#
> +# Properties for filter-rewriter objects.
> +#
> +# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'FilterRewriterProperties',
> +  'base': 'NetfilterProperties',
> +  'data': { '*vnet_hdr_support': 'bool' } }
> +

Matches net/filter-rewriter.c:filter_rewriter_class_init().

>  ##
>  # @IothreadProperties:
>  #
> @@ -508,6 +639,12 @@
>      'cryptodev-backend-builtin',
>      'cryptodev-vhost-user',
>      'dbus-vmstate',
> +    'filter-buffer',
> +    'filter-dump',
> +    'filter-mirror',
> +    'filter-redirector',
> +    'filter-replay',
> +    'filter-rewriter',
>      'iothread',
>      'memory-backend-file',
>      'memory-backend-memfd',
> @@ -550,6 +687,12 @@
>        'cryptodev-backend-builtin':  'CryptodevBackendProperties',
>        'cryptodev-vhost-user':       'CryptodevVhostUserProperties',
>        'dbus-vmstate':               'DBusVMStateProperties',
> +      'filter-buffer':              'FilterBufferProperties',
> +      'filter-dump':                'FilterDumpProperties',
> +      'filter-mirror':              'FilterMirrorProperties',
> +      'filter-redirector':          'FilterRedirectorProperties',
> +      'filter-replay':              'NetfilterProperties',
> +      'filter-rewriter':            'FilterRewriterProperties',
>        'iothread':                   'IothreadProperties',
>        'memory-backend-file':        'MemoryBackendFileProperties',
>        'memory-backend-memfd':       'MemoryBackendMemfdProperties',
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org