> -----Original Message-----
> From: Markus Armbruster <arm...@redhat.com>
> Sent: Friday, June 11, 2021 5:37 PM
> To: Zhang, Chen <chen.zh...@intel.com>
> Cc: Eric Blake <ebl...@redhat.com>; Lukas Straub <lukasstra...@web.de>;
> Daniel P.Berrangé <berra...@redhat.com>; Li Zhijian
> <lizhij...@cn.fujitsu.com>; Jason Wang <jasow...@redhat.com>; Markus
> Armbruster <arm...@redhat.com>; qemu-dev <qemu-devel@nongnu.org>;
> Gerd Hoffmann <kra...@redhat.com>; Zhang Chen <zhangc...@gmail.com>;
> Dr. David Alan Gilbert <dgilb...@redhat.com>
> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command
> for COLO passthrough
>
> "Zhang, Chen" <chen.zh...@intel.com> writes:
>
> >> -----Original Message-----
> >> From: Eric Blake <ebl...@redhat.com>
> >> Sent: Friday, June 4, 2021 10:35 PM
> >> To: Zhang, Chen <chen.zh...@intel.com>
> >> Cc: Jason Wang <jasow...@redhat.com>; qemu-dev <qemu-
> >> de...@nongnu.org>; Dr. David Alan Gilbert <dgilb...@redhat.com>;
> >> Markus Armbruster <arm...@redhat.com>; Daniel P. Berrangé
> >> <berra...@redhat.com>; Gerd Hoffmann <kra...@redhat.com>; Li
> Zhijian
> >> <lizhij...@cn.fujitsu.com>; Zhang Chen <zhangc...@gmail.com>; Lukas
> >> Straub <lukasstra...@web.de>
> >> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP
> command
> >> for COLO passthrough
> >>
> >> On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
> >> > Since the real user scenario does not need COLO to monitor all traffic.
> >> > Add colo-passthrough-add and colo-passthrough-del to maintain a
> >> > COLO network passthrough list. Add IPFlowSpec struct for all QMP
> commands.
> >> > Except protocol field is necessary, other fields are optional.
> >>
> >> That last sentence reads awkwardly, and I don't see a protocol field
> >> in the patch below.
> >
> > Oh, We move the protocol field to optional by Lukas's comments in V6.
> > I will remove this comments here.
> >
> >>
> >> >
> >> > Signed-off-by: Zhang Chen <chen.zh...@intel.com>
> >> > ---
> >> > net/net.c | 10 ++++++++
> >> > qapi/net.json | 68
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >> > 2 files changed, 78 insertions(+)
> >> >
> >>
> >> > +++ b/qapi/net.json
> >> > @@ -7,6 +7,7 @@
> >> > ##
> >> >
> >> > { 'include': 'common.json' }
> >> > +{ 'include': 'sockets.json' }
> >> >
> >> > ##
> >> > # @set_link:
> >> > @@ -694,3 +695,70 @@
> >> > ##
> >> > { 'event': 'FAILOVER_NEGOTIATED',
> >> > 'data': {'device-id': 'str'} }
> >> > +
> >> > +##
> >> > +# @IPFlowSpec:
> >> > +#
> >> > +# IP flow specification.
> >> > +#
> >> > +# @protocol: Transport layer protocol like TCP/UDP...
> >>
> >> Why is this open-coded as 'str' instead of an enum?
> >
> > The original code use enum, but we change it by Dave and Markus's
> comments.
> > Please check the history:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03919.html
>
> It's a string to be passed to getprotobyname(3). Please mention that in the
> doc string.
OK, I will add this to commit log and qapi/net.json in next version.
>
> It's not an enum, because we don't want to duplicate /etc/protocols in the
> QAPI schema.
Yes.
>
> >> > +#
> >> > +# @object-name: Point out the IPflow spec effective range of
> >> > +object,
>
> I have no idea what that means :)
>
> It might be what was called @id in v4. There, you explained
>
> The @id means packet hander in Qemu. Because not all the guest network
> packet into the colo-compare module, the net-filters are same cases.
> There modules attach to NIC or chardev socket to work, VM maybe have
> multi modules running. So we use the ID to set the rule to the specific
> module.
>
> and I asked you to work it into the doc comment.
>
Yes, As we discussed In V4, already changed the "id" to "object-name".
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01088.html
And sorry I missed another mail thread to add the comments, I will add the
explanation and quick update to V8 for this series.
Thanks for your help~
Thanks
Chen
> If you want help with working it into the doc comment, please explain its
> intended use for dummies :)
>
> >> > +# If there is no such part, it means global spec.
> >> > +#
> >> > +# @source: Source address and port.
> >> > +#
> >> > +# @destination: Destination address and port.
> >> > +#
> >> > +# Since: 6.1
> >> > +##
> >> > +{ 'struct': 'IPFlowSpec',
> >> > + 'data': { '*protocol': 'str', '*object-name': 'str',
> >> > + '*source': 'InetSocketAddressBase',
> >> > + '*destination': 'InetSocketAddressBase' } }
> >> > +
> >> > +##
> >> > +# @colo-passthrough-add:
> >> > +#
> >> > +# Add passthrough entry according to user's needs in COLO-compare.
> >> > +# Source IP/port and destination IP/port both optional, If user
> >> > +just # input parts of infotmation, it will match all.
> >>
> >> information
> >>
> >> Grammar suggestion:
> >>
> >> The source and destination IP/ports are both optional; if the user
> >> only inputs part of the information, this will match all traffic.
> >>
> >> except I'm not sure if my rewrite conveys the actual intent.
> >
> > Looks good to me, It should add the "protocol" to optional too.
> > Sorry, I'm not a native speaker, I will fix it in next version.
> >
> >>
> >> > +#
> >> > +# Returns: Nothing on success
> >> > +#
> >> > +# Since: 6.1
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "colo-passthrough-add",
> >> > +# "arguments": { "protocol": "tcp", "object-name": "object0",
> >> > +# "source": {"host": "192.168.1.1", "port": "1234"},
> >> > +# "destination": {"host": "192.168.1.2", "port": "4321"} } }
> >> > +# <- { "return": {} }
> >> > +#
> >> > +##
> >> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
> >> > + 'data': 'IPFlowSpec' }
> >> > +
> >> > +##
> >> > +# @colo-passthrough-del:
> >> > +#
> >> > +# Delete passthrough entry according to user's needs in COLO-
> compare.
> >> > +# Source IP/port and destination IP/port both optional, If user
> >> > +just # input parts of infotmation, it will match all.
> >>
> >> Same problems as above.
> >
> > OK.
> >
> > Thanks
> > Chen
> >
> >>
> >> > +#
> >> > +# Returns: Nothing on success
> >> > +#
> >> > +# Since: 6.1
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "colo-passthrough-del",
> >> > +# "arguments": { "protocol": "tcp", "object-name": "object0",
> >> > +# "source": {"host": "192.168.1.1", "port": "1234"},
> >> > +# "destination": {"host": "192.168.1.2", "port": "4321"} } }
> >> > +# <- { "return": {} }
> >> > +#
> >> > +##
> >> > +{ 'command': 'colo-passthrough-del', 'boxed': true,
> >> > + 'data': 'IPFlowSpec' }
> >> > --
> >> > 2.25.1
> >> >
> >>
> >> --
> >> Eric Blake, Principal Software Engineer
> >> Red Hat, Inc. +1-919-301-3266
> >> Virtualization: qemu.org | libvirt.org
