RE: [PATCH V4 2/7] qapi/net.json: Add L4_Connection definition

["Zhang, Chen" <chen.zhang@intel.com>]

> -----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