RE: [PATCH V4 3/7] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Saturday, March 20, 2021 12:03 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>; Lukas
> Straub <lukasstraub2@web.de>; Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH V4 3/7] qapi/net: Add new QMP command for COLO
> passthrough
> 
> Zhang Chen <chen.zhang@intel.com> writes:
> 
> > 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.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/net.c     | 10 ++++++++++
> >  qapi/net.json | 40 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 725a4e1450..7c7cefe0e0 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1199,6 +1199,16 @@ void qmp_netdev_del(const char *id, Error
> **errp)
> >      }
> >  }
> >
> > +void qmp_colo_passthrough_add(L4_Connection *conn, Error **errp) {
> > +    /* Setup passthrough connection */
> 
> Do you mean to say
> 
>        /* TODO implement */
> 
> ?

Yes, I will input real code here in 7/7 patch.

> 
> > +}
> > +
> > +void qmp_colo_passthrough_del(L4_Connection *conn, Error **errp) {
> > +    /* Delete passthrough connection */ }
> 
> Likewise.
> 
> > +
> >  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)  {
> >      char *str;
> > diff --git a/qapi/net.json b/qapi/net.json index
> > cd4a8ed95e..ec7d3b1128 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -851,3 +851,43 @@
> >    'data': { 'protocol': 'IP_PROTOCOL', '*id': 'str', '*src_ip': 'str', 
> > '*dst_ip': 'str',
> >      '*src_port': 'int', '*dst_port': 'int' } }
> >
> > +##
> > +# @colo-passthrough-add:
> > +#
> > +# Add passthrough entry according to customer's needs in COLO-compare.
> 
> QEMU doesn't have customers, it has users :)

Thanks note.

> 
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 6.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-add",
> > +#      "arguments": { "protocol": "tcp", "id": "object0", "src_ip":
> "192.168.1.1",
> > +#      "dst_ip": "192.168.1.2", "src_port": 1234, "dst_port": 4321 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
> > +     'data': 'L4_Connection' }
> > +
> > +##
> > +# @colo-passthrough-del:
> > +#
> > +# Delete passthrough entry according to customer's needs in COLO-
> compare.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 6.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-del",
> > +#      "arguments": { "protocol": "tcp", "id": "object0", "src_ip":
> "192.168.1.1",
> > +#      "dst_ip": "192.168.1.2", "src_port": 1234, "dst_port": 4321 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-del', 'boxed': true,
> > +     'data': 'L4_Connection' }
> > +
> 
> To make sense of this, I have to refer back to PATCH 1 and 2:
> 
>    { 'enum': 'IP_PROTOCOL', 'data': [ 'tcp', 'udp', 'dccp', 'sctp', 'udplite',
>        'icmp', 'igmp', 'ipv6' ] }
> 
>    { 'struct': 'L4_Connection',
>      'data': { 'protocol': 'IP_PROTOCOL', '*id': 'str', '*src_ip': 'str', 
> '*dst_ip': 'str',
>        '*src_port': 'int', '*dst_port': 'int' } }
> 
> Please squash the three patches together.

OK.

> 
> I figure colo-passthrough-add adds some kind of packet matching thingy that
> can match packets by source IP, source port, destination IP, destination port,
> and protocol.  Correct?

Yes, you are right.

> 
> The protocol is mandatory, all others are optional.  What does it mean to omit
> an optional one?  Match all?

Yes, match all. The idea from Jason Wang, for example:
User just set the protocol/source IP(tcp/192.168.1.1) , others empty.
The rule will bypass all the TCP packet from the source IP.

> 
> I have no idea what @id is supposed to mean.  Please explain intended use.

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. 

Thanks
Chen

> 
> I'm ignoring colo-passthrough-del for now, because I feel need to
> understand -add first.