[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] Unified Datagram Socket Transport - GRE sup
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] Unified Datagram Socket Transport - GRE support |
Date: |
Wed, 19 Jul 2017 09:40:59 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/18/2017 12:08 PM, address@hidden wrote:
> From: Anton Ivanov <address@hidden>
>
> This adds GRETAP support to the unified socket driver.
>
> Signed-off-by: Anton Ivanov <address@hidden>
> ---
> net/Makefile.objs | 2 +-
> net/clients.h | 4 +
> net/gre.c | 313
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> net/net.c | 5 +
> qapi-schema.json | 46 +++++++-
> qemu-options.hx | 63 ++++++++++-
> 6 files changed, 425 insertions(+), 8 deletions(-)
> create mode 100644 net/gre.c
>
Just an interface review:
> +++ b/qapi-schema.json
> @@ -3847,7 +3847,41 @@
> 'txsession': 'uint32',
> '*rxsession': 'uint32',
> '*offset': 'uint32' } }
> -
> +##
> +# @NetdevGREOptions:
> +#
> +# Connect the VLAN to Ethernet over Ethernet over GRE (GRETAP) tunnel
> +#
> +# @src: source address
> +#
> +# @dst: destination address
> +#
> +# @ipv6: force the use of ipv6
This doesn't quite match what we do with other sockets (where we have
both ipv4 and ipv6 booleans to allow IPv4-only, IPv6-only, or both). Is
this something where we can reuse InetSocketAddress instead of inventing
yet another way of doing things?
Then again, it does match what NetdevL2TPv3Options did :(
> +#
> +# @sequence: have sequence counter
> +#
> +# @pinsequence: pin sequence counter to zero -
> +# workaround for buggy implementations or
> +# networks with packet reorder
> +#
> +# @txkey: 32 bit transmit key
> +#
> +# @rxkey: 32 bit receive key
Worth listing what the defaults are for these optional fields when not
present?
> +#
> +# Note - gre checksums are not supported at present
> +#
> +#
> +# Since 2.9
You've missed 2.9 by a long shot. You've also missed 2.10 softfreeze
for a new feature, so this should read since 2.11.
> @@ -3966,7 +4000,7 @@
> ##
> { 'enum': 'NetClientDriver',
> 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
> - 'bridge', 'hubport', 'netmap', 'vhost-user' ] }
> + 'bridge', 'hubport', 'netmap', 'vhost-user', 'gre' ] }
Worth adding a comment that 'gre' is since 2.11 (look for other enums
that have had additions after the initial release of the enum, for an
example of the preferred format).
>
> ##
> # @Netdev:
> @@ -3996,7 +4030,8 @@
> 'bridge': 'NetdevBridgeOptions',
> 'hubport': 'NetdevHubPortOptions',
> 'netmap': 'NetdevNetmapOptions',
> - 'vhost-user': 'NetdevVhostUserOptions' } }
> + 'vhost-user': 'NetdevVhostUserOptions',
> + 'gre': 'NetdevGREOptions' } }
Okay.
>
> ##
> # @NetLegacy:
> @@ -4027,7 +4062,7 @@
> ##
> { 'enum': 'NetLegacyOptionsType',
> 'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> - 'dump', 'bridge', 'netmap', 'vhost-user'] }
> + 'dump', 'bridge', 'netmap', 'vhost-user', 'gre'] }
Not okay. NetLegacy should never grow again (that's the whole point of
it being legacy - we're trying to phase it out).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 1/3] Unified Datagram Socket Transport, anton . ivanov, 2017/07/18
Re: [Qemu-devel] [PATCH 1/3] Unified Datagram Socket Transport, Anton Ivanov, 2017/07/21