"Zhang, Chen" <chen.zh...@intel.com> writes:
>> -----Original Message-----
>> From: Markus Armbruster <arm...@redhat.com>
>> Sent: Saturday, March 20, 2021 12:03 AM
>> To: Zhang, Chen <chen.zh...@intel.com>
>> Cc: Jason Wang <jasow...@redhat.com>; qemu-dev <qemu-
>> de...@nongnu.org>; Eric Blake <ebl...@redhat.com>; Dr. David Alan
>> Gilbert <dgilb...@redhat.com>; Li Zhijian <lizhij...@cn.fujitsu.com>; Lukas
>> Straub <lukasstra...@web.de>; Zhang Chen <zhangc...@gmail.com>
>> Subject: Re: [PATCH V4 3/7] qapi/net: Add new QMP command for COLO
>> passthrough
>>
>> Zhang Chen <chen.zh...@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.zh...@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.
Use a TODO comment then.
>>
>> > +}
>> > +
>> > +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.
Work this into the doc comment, please.
>> 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.
I'm not sure I understand, but then I'm a QEMU networking ignoramus :)
Work it into the doc comment.
> Thanks
> Chen
>
>>
>> I'm ignoring colo-passthrough-del for now, because I feel need to
>> understand -add first.
