Markus Armbruster <arm...@redhat.com> writes: > I see Jason queued this while I was failing at keeping up with review. > I apologize for dropping the ball. > > There still are a few unresolved issues I raised in prior review. The > documentation is not ready, and there is no consensus on the design of > passthrough-filter-del. > > If we merge this as is for 6.2, then I want my review to be addressed on > top.
One more thing... > Zhang Chen <chen.zh...@intel.com> writes: > >> Since the real user scenario does not need to monitor all traffic. >> Add passthrough-filter-add and passthrough-filter-del to maintain >> a network passthrough list in object with network packet processing >> function. Add IPFlowSpec struct for all QMP commands. >> Most the fields of IPFlowSpec are optional,except object-name. >> >> Signed-off-by: Zhang Chen <chen.zh...@intel.com> >> --- > > [...] > >> diff --git a/qapi/net.json b/qapi/net.json >> index 7fab2e7cd8..bfe38faab5 100644 >> --- a/qapi/net.json >> +++ b/qapi/net.json >> @@ -7,6 +7,7 @@ >> ## >> >> { 'include': 'common.json' } >> +{ 'include': 'sockets.json' } >> >> ## >> # @set_link: >> @@ -696,3 +697,80 @@ >> ## >> { 'event': 'FAILOVER_NEGOTIATED', >> 'data': {'device-id': 'str'} } >> + >> +## >> +# @IPFlowSpec: >> +# >> +# IP flow specification. >> +# >> +# @protocol: Transport layer protocol like TCP/UDP, etc. The protocol is the >> +# string instead of enum, because it can be passed to >> getprotobyname(3) >> +# and avoid duplication with /etc/protocols. > > In review of v8, I wrote: > > The rationale is good, but it doesn't really belong into the interface > documentation. Suggest: > > # @protocol: Transport layer protocol like TCP/UDP, etc. This will be > # passed to getprotobyname(3). > > to which you replied "OK." Please apply the change. > >> +# >> +# @object-name: The @object-name means a qemu object with network packet >> +# processing function, for example colo-compare, >> filtr-redirector >> +# filtr-mirror, etc. VM can running with multi network packet > > s/qemu/QEMU/ > > s/filtr/filter/ two times here, and more below. > > s/VM/The VM/ > > s/multi/multiple/ > > Also, limit doc comment line length to 70 characters or so. > >> +# processing function objects. They can control different >> network >> +# data paths from netdev or chardev. So it needs the >> object-name >> +# to set the effective module. > > Again, this is rationale, which doesn't really belong here. > > What does belong here, but isn't: meaning of @object-name, i.e. how it > is resolved to a "qemu object with network packet processing function", > whatever that is. > > I'm *guessing* it's the QOM path of a QOM object that provides a certain > interface. Correct? > > If yes, which interface exactly? Is it a QOM interface? > > The comment could then look like > > # QOM path to a QOM object that implements the MUMBLE interface. > > with the details corrected / fleshed out. > >> +# >> +# @source: Source address and port. >> +# >> +# @destination: Destination address and port. >> +# >> +# Since: 6.1 6.2 now. More of the same below. [...]