[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition
From: |
Zhang, Chen |
Subject: |
RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition |
Date: |
Tue, 23 Mar 2021 09:06:13 +0000 |
> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Monday, March 22, 2021 8:31 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Lukas Straub <lukasstraub2@web.de>; Li Zhijian
> <lizhijian@cn.fujitsu.com>; Jason Wang <jasowang@redhat.com>; qemu-
> dev <qemu-devel@nongnu.org>; Dr. David Alan Gilbert
> <dgilbert@redhat.com>; Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition
>
> "Zhang, Chen" <chen.zhang@intel.com> writes:
>
> >> -----Original Message-----
> >> From: Markus Armbruster <armbru@redhat.com>
> >> Sent: Friday, March 19, 2021 11:48 PM
> >> 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>; Markus Armbruster
> <armbru@redhat.com>;
> >> Li Zhijian <lizhijian@cn.fujitsu.com>; Lukas Straub
> >> <lukasstraub2@web.de>; Zhang Chen <zhangckid@gmail.com>
> >> Subject: Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection
> >> definition
> >>
> >> Zhang Chen <chen.zhang@intel.com> writes:
> >>
> >> > Add L4_Connection struct for other QMP commands.
> >> > Except protocol field is necessary, other fields are optional.
> >> >
> >> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> >> > ---
> >> > qapi/net.json | 26 ++++++++++++++++++++++++++
> >> > 1 file changed, 26 insertions(+)
> >> >
> >> > diff --git a/qapi/net.json b/qapi/net.json index
> >> > 498ea7aa72..cd4a8ed95e 100644
> >> > --- a/qapi/net.json
> >> > +++ b/qapi/net.json
> >> > @@ -825,3 +825,29 @@
> >> > { 'enum': 'IP_PROTOCOL', 'data': [ 'tcp', 'udp', 'dccp', 'sctp',
> >> > 'udplite',
> >> > 'icmp', 'igmp', 'ipv6' ] }
> >> >
> >> > +##
> >> > +# @L4_Connection:
> >> > +#
> >> > +# Layer 4 network connection.
> >> > +#
> >> > +# Just for IPv4.
> >> > +#
> >> > +# @protocol: Transport layer protocol like TCP/UDP...
> >> > +#
> >> > +# @id: For specific module with Qemu object ID, If there is no such
> >> > part,
> >> > +# it means global rules.
> >>
> >> Clear as mud.
> >
> > Sorry, let me re-clear it.
> > If I understand correctly, The ID shouldn't be here, but I found the 'boxed'
> flag just can add only one 'data' like this:
> > +##
> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
> > + 'data': 'L4_Connection' }
> >
> > I original want to this:
> > +##
> > +{ 'command': 'colo-passthrough-add',
> > + 'data': { 'id': 'str', 'boxed': false, 'conn': 'L4_Connection',
> > +'boxed': true }
> >
> > So, I add the @id as an optional argument here.
> >
> > rewrite the comments:
> > +# @id: Assign the rule to Qemu network handle module object ID. Like
> colo-compare, net-filter.
> >
> > Please see the ID details in patch3 too.
>
> So, colo-passthrough-add takes an @id argument (to be tacked onto packets
> to help with further processing, I understand), and arguments to match
> packets.
Yes.
>
> Naming the argument type L4_Connection is misleading.
>
> Even naming the match arguments L4_Connection would be misleading.
> "Connection" has a specific meaning in networking. There are TCP
> connections. There is no such thing as an UDP connection.
>
> A TCP connection is uniquely identified by a pair of endpoints, i.e. by source
> address, source port, destination address, destination port.
> Same for other connection-oriented protocols. The protocol is not part of
> the connection. Thus, L4_Connection would be misleading even for the
> connection-oriented case.
>
> You need a named type for colo-passthrough-add's argument because you
> share it with colo-passthrough-del. I'm not sure that's what we want (I'm
> going to write more on that in a moment). If it is what we want, then please
> pick a another, descriptive name.
What do you think the "L4BypassRule" or "NetworkRule" ?
Thanks
Chen
- [PATCH V4 1/7] qapi/net.json: Add IP_PROTOCOL definition, (continued)
- [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Zhang Chen, 2021/03/19
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/19
- RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Zhang, Chen, 2021/03/22
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/22
- RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition,
Zhang, Chen <=
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/23
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Dr. David Alan Gilbert, 2021/03/23
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/24
- Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/24
- RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Zhang, Chen, 2021/03/25
- RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Zhang, Chen, 2021/03/23
Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/19
Re: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition, Markus Armbruster, 2021/03/24
[PATCH V4 3/7] qapi/net: Add new QMP command for COLO passthrough, Zhang Chen, 2021/03/19