| Message ID | 6643a807a5f4c5ceb2f09e0718e97e5f968577f7.1565894837.git.lukasstraub2@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | colo: Add support for continious replication | expand |
> -----Original Message----- > From: Lukas Straub [mailto:lukasstraub2@web.de] > Sent: Friday, August 16, 2019 2:49 AM > To: qemu-devel <qemu-devel@nongnu.org> > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>; Xie > Changlong <xiechanglong.d@gmail.com> > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in > the filter list > > To switch the Secondary to Primary, we need to insert new filters before the > filter-rewriter. > > Add the options insert= and position= to be able to insert filters anywhere in the > filter list. > > position should be either "head", "tail" or the id of another filter. > insert should be either "before" or "after" to specify where to insert the new > filter relative to the one specified with position. > Hi Lukas, It looks no need to add the "insert = xxx" for this operation. For example: We have 3 net-filters, the running order like that: Fiter1 ----------> Filter2 ------------> Filter3 If we want to add another filter between filter1 and filter2. The "Position = head, insert = after" always seam with "position = filter2 id, insert = before". It seems the "insert" is a redundant args. So I think it is enough with the "position", we can make the "insert" always equal "after" except the "head". Thanks Zhang Chen > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > include/net/filter.h | 2 ++ > net/filter.c | 71 +++++++++++++++++++++++++++++++++++++++++++- > qemu-options.hx | 10 +++---- > 3 files changed, 77 insertions(+), 6 deletions(-) > > diff --git a/include/net/filter.h b/include/net/filter.h index > 49da666ac0..355c178f75 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -62,6 +62,8 @@ struct NetFilterState { > NetClientState *netdev; > NetFilterDirection direction; > bool on; > + char *position; > + bool insert_before; > QTAILQ_ENTRY(NetFilterState) next; > }; > > diff --git a/net/filter.c b/net/filter.c index 28d1930db7..309fd778df 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -171,11 +171,47 @@ static void netfilter_set_status(Object *obj, const > char *str, Error **errp) > } > } > > +static char *netfilter_get_position(Object *obj, Error **errp) { > + NetFilterState *nf = NETFILTER(obj); > + > + return g_strdup(nf->position); > +} > + > +static void netfilter_set_position(Object *obj, const char *str, Error > +**errp) { > + NetFilterState *nf = NETFILTER(obj); > + > + nf->position = g_strdup(str); > +} > + > +static char *netfilter_get_insert(Object *obj, Error **errp) { > + NetFilterState *nf = NETFILTER(obj); > + > + return nf->insert_before ? g_strdup("before") : g_strdup("after"); > +} > + > +static void netfilter_set_insert(Object *obj, const char *str, Error > +**errp) { > + NetFilterState *nf = NETFILTER(obj); > + > + if (strcmp(str, "before") && strcmp(str, "after")) { > + error_setg(errp, "Invalid value for netfilter insert, " > + "should be 'head' or 'tail'"); > + return; > + } > + > + nf->insert_before = !strcmp(str, "before"); } > + > static void netfilter_init(Object *obj) { > NetFilterState *nf = NETFILTER(obj); > > nf->on = true; > + nf->insert_before = false; > + nf->position = g_strdup("tail"); > > object_property_add_str(obj, "netdev", > netfilter_get_netdev_id, netfilter_set_netdev_id, @@ -187,11 > +223,18 @@ static void netfilter_init(Object *obj) > object_property_add_str(obj, "status", > netfilter_get_status, netfilter_set_status, > NULL); > + object_property_add_str(obj, "position", > + netfilter_get_position, netfilter_set_position, > + NULL); > + object_property_add_str(obj, "insert", > + netfilter_get_insert, netfilter_set_insert, > + NULL); > } > > static void netfilter_complete(UserCreatable *uc, Error **errp) { > NetFilterState *nf = NETFILTER(uc); > + NetFilterState *position = NULL; > NetClientState *ncs[MAX_QUEUE_NUM]; > NetFilterClass *nfc = NETFILTER_GET_CLASS(uc); > int queues; > @@ -219,6 +262,20 @@ static void netfilter_complete(UserCreatable *uc, > Error **errp) > return; > } > > + if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) { > + /* Search for the position to insert before/after */ > + Object *container; > + Object *obj; > + > + container = object_get_objects_root(); > + obj = object_resolve_path_component(container, nf->position); > + if (!obj) { > + error_setg(errp, "filter '%s' not found", nf->position); > + return; > + } > + position = NETFILTER(obj); > + } > + > nf->netdev = ncs[0]; > > if (nfc->setup) { > @@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc, > Error **errp) > return; > } > } > - QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > + > + if (position) { > + if (nf->insert_before) { > + QTAILQ_INSERT_BEFORE(position, nf, next); > + } else { > + QTAILQ_INSERT_AFTER(&nf->netdev->filters, position, nf, next); > + } > + } else if (!strcmp(nf->position, "head")) { > + QTAILQ_INSERT_HEAD(&nf->netdev->filters, nf, next); > + } else if (!strcmp(nf->position, "tail")) { > + QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > + } > } > > static void netfilter_finalize(Object *obj) @@ -245,6 +313,7 @@ static void > netfilter_finalize(Object *obj) > QTAILQ_REMOVE(&nf->netdev->filters, nf, next); > } > g_free(nf->netdev_id); > + g_free(nf->position); > } > > static void default_handle_event(NetFilterState *nf, int event, Error **errp) > diff --git a/qemu-options.hx b/qemu-options.hx index 08749a3391..f0a47a0746 > 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4368,7 +4368,7 @@ applications, they can do this through this parameter. > Its format is a gnutls priority string as described at > @url{https://gnutls.org/manual/html_node/Priority-Strings.html}. > > -@item -object filter- > buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|r > x|tx}][,status=@var{on|off}] > +@item -object > +filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue > +=@var{all|rx|tx}][,status=@var{on|off}][,position=@var{head|tail|id}][, > +insert=@var{after|before}] > > Interval @var{t} can't be 0, this filter batches the packet delivery: all packets > arriving in a given interval on netdev @var{netdevid} are delayed @@ -4387,11 > +4387,11 @@ queue @var{all|rx|tx} is an option that can be applied to any > netfilter. > @option{tx}: the filter is attached to the transmit queue of the netdev, > where it will receive packets sent by the netdev. > > -@item -object filter- > mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@v > ar{all|rx|tx}[,vnet_hdr_support] > +@item -object > +filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}, > +queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id}][ > +,insert=@var{after|before}] > > filter-mirror on netdev @var{netdevid},mirror net packet to > chardev@var{chardevid}, if it has the vnet_hdr_support flag, filter-mirror will > mirror packet with vnet_hdr_len. > > -@item -object filter- > redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev= > @var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support] > +@item -object > +filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevi > +d},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,pos > +ition=@var{head|tail|id}][,insert=@var{after|before}] > > filter-redirector on netdev @var{netdevid},redirect filter's net packet to > chardev @var{chardevid},and redirect indev's packet to filter.if it has the > vnet_hdr_support flag, @@ -4400,7 +4400,7 @@ Create a filter-redirector we > need to differ outdev id from indev id, id can not be the same. we can just use > indev or outdev, but at least one of indev or outdev need to be specified. > > -@item -object filter- > rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_ > support] > +@item -object > +filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx} > +,[vnet_hdr_support][,position=@var{head|tail|id}][,insert=@var{after|be > +fore}] > > Filter-rewriter is a part of COLO project.It will rewrite tcp packet to secondary > from primary to keep secondary tcp connection,and rewrite @@ -4413,7 > +4413,7 @@ colo secondary: > -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 > -object filter-rewriter,id=rew0,netdev=hn0,queue=all > > -@item -object filter- > dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}] > +@item -object > +filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen= > +@var{len}][,position=@var{head|tail|id}][,insert=@var{after|before}] > > Dump the network traffic on netdev @var{dev} to the file specified by > @var{filename}. At most @var{len} bytes (64k by default) per packet are stored. > -- > 2.20.1
On Fri, 23 Aug 2019 03:24:02 +0000 "Zhang, Chen" <chen.zhang@intel.com> wrote: > > -----Original Message----- > > From: Lukas Straub [mailto:lukasstraub2@web.de] > > Sent: Friday, August 16, 2019 2:49 AM > > To: qemu-devel <qemu-devel@nongnu.org> > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>; Xie > > Changlong <xiechanglong.d@gmail.com> > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in > > the filter list > > > > To switch the Secondary to Primary, we need to insert new filters before the > > filter-rewriter. > > > > Add the options insert= and position= to be able to insert filters anywhere in the > > filter list. > > > > position should be either "head", "tail" or the id of another filter. > > insert should be either "before" or "after" to specify where to insert the new > > filter relative to the one specified with position. > > > > Hi Lukas, > > It looks no need to add the "insert = xxx" for this operation. > For example: > > We have 3 net-filters, the running order like that: > > Fiter1 ----------> Filter2 ------------> Filter3 > > If we want to add another filter between filter1 and filter2. > The "Position = head, insert = after" always seam with "position = filter2 id, insert = before". Hi Zhang, The insert= parameter is ignored if position=head or tail. It always Inserts at the head (before Filter1) or the tail (after Filter3) of the List in these cases. > It seems the "insert" is a redundant args. > So I think it is enough with the "position", we can make the "insert" always equal "after" except the "head". Yes, we still could do it without it, but its more convenient with the insert= parameter. For example our Case with inserting before the rewriter: 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } You see directly that here 3 Filters are inserted before the rewriter. would have to become: 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } Which is less obvious. Regards, Lukas Straub > > Thanks > Zhang Chen > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > --- > > include/net/filter.h | 2 ++ > > net/filter.c | 71 +++++++++++++++++++++++++++++++++++++++++++- > > qemu-options.hx | 10 +++---- > > 3 files changed, 77 insertions(+), 6 deletions(-) > > > > diff --git a/include/net/filter.h b/include/net/filter.h index > > 49da666ac0..355c178f75 100644 > > --- a/include/net/filter.h > > +++ b/include/net/filter.h > > @@ -62,6 +62,8 @@ struct NetFilterState { > > NetClientState *netdev; > > NetFilterDirection direction; > > bool on; > > + char *position; > > + bool insert_before; > > QTAILQ_ENTRY(NetFilterState) next; > > }; > > > > diff --git a/net/filter.c b/net/filter.c index 28d1930db7..309fd778df 100644 > > --- a/net/filter.c > > +++ b/net/filter.c > > @@ -171,11 +171,47 @@ static void netfilter_set_status(Object *obj, const > > char *str, Error **errp) > > } > > } > > > > +static char *netfilter_get_position(Object *obj, Error **errp) { > > + NetFilterState *nf = NETFILTER(obj); > > + > > + return g_strdup(nf->position); > > +} > > + > > +static void netfilter_set_position(Object *obj, const char *str, Error > > +**errp) { > > + NetFilterState *nf = NETFILTER(obj); > > + > > + nf->position = g_strdup(str); > > +} > > + > > +static char *netfilter_get_insert(Object *obj, Error **errp) { > > + NetFilterState *nf = NETFILTER(obj); > > + > > + return nf->insert_before ? g_strdup("before") : g_strdup("after"); > > +} > > + > > +static void netfilter_set_insert(Object *obj, const char *str, Error > > +**errp) { > > + NetFilterState *nf = NETFILTER(obj); > > + > > + if (strcmp(str, "before") && strcmp(str, "after")) { > > + error_setg(errp, "Invalid value for netfilter insert, " > > + "should be 'head' or 'tail'"); > > + return; > > + } > > + > > + nf->insert_before = !strcmp(str, "before"); } > > + > > static void netfilter_init(Object *obj) { > > NetFilterState *nf = NETFILTER(obj); > > > > nf->on = true; > > + nf->insert_before = false; > > + nf->position = g_strdup("tail"); > > > > object_property_add_str(obj, "netdev", > > netfilter_get_netdev_id, netfilter_set_netdev_id, @@ -187,11 > > +223,18 @@ static void netfilter_init(Object *obj) > > object_property_add_str(obj, "status", > > netfilter_get_status, netfilter_set_status, > > NULL); > > + object_property_add_str(obj, "position", > > + netfilter_get_position, netfilter_set_position, > > + NULL); > > + object_property_add_str(obj, "insert", > > + netfilter_get_insert, netfilter_set_insert, > > + NULL); > > } > > > > static void netfilter_complete(UserCreatable *uc, Error **errp) { > > NetFilterState *nf = NETFILTER(uc); > > + NetFilterState *position = NULL; > > NetClientState *ncs[MAX_QUEUE_NUM]; > > NetFilterClass *nfc = NETFILTER_GET_CLASS(uc); > > int queues; > > @@ -219,6 +262,20 @@ static void netfilter_complete(UserCreatable *uc, > > Error **errp) > > return; > > } > > > > + if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) { > > + /* Search for the position to insert before/after */ > > + Object *container; > > + Object *obj; > > + > > + container = object_get_objects_root(); > > + obj = object_resolve_path_component(container, nf->position); > > + if (!obj) { > > + error_setg(errp, "filter '%s' not found", nf->position); > > + return; > > + } > > + position = NETFILTER(obj); > > + } > > + > > nf->netdev = ncs[0]; > > > > if (nfc->setup) { > > @@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc, > > Error **errp) > > return; > > } > > } > > - QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > > + > > + if (position) { > > + if (nf->insert_before) { > > + QTAILQ_INSERT_BEFORE(position, nf, next); > > + } else { > > + QTAILQ_INSERT_AFTER(&nf->netdev->filters, position, nf, next); > > + } > > + } else if (!strcmp(nf->position, "head")) { > > + QTAILQ_INSERT_HEAD(&nf->netdev->filters, nf, next); > > + } else if (!strcmp(nf->position, "tail")) { > > + QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > > + } > > } > > > > static void netfilter_finalize(Object *obj) @@ -245,6 +313,7 @@ static void > > netfilter_finalize(Object *obj) > > QTAILQ_REMOVE(&nf->netdev->filters, nf, next); > > } > > g_free(nf->netdev_id); > > + g_free(nf->position); > > } > > > > static void default_handle_event(NetFilterState *nf, int event, Error **errp) > > diff --git a/qemu-options.hx b/qemu-options.hx index 08749a3391..f0a47a0746 > > 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -4368,7 +4368,7 @@ applications, they can do this through this parameter. > > Its format is a gnutls priority string as described at > > @url{https://gnutls.org/manual/html_node/Priority-Strings.html}. > > > > -@item -object filter- > > buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|r > > x|tx}][,status=@var{on|off}] > > +@item -object > > +filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue > > +=@var{all|rx|tx}][,status=@var{on|off}][,position=@var{head|tail|id}][, > > +insert=@var{after|before}] > > > > Interval @var{t} can't be 0, this filter batches the packet delivery: all packets > > arriving in a given interval on netdev @var{netdevid} are delayed @@ -4387,11 > > +4387,11 @@ queue @var{all|rx|tx} is an option that can be applied to any > > netfilter. > > @option{tx}: the filter is attached to the transmit queue of the netdev, > > where it will receive packets sent by the netdev. > > > > -@item -object filter- > > mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@v > > ar{all|rx|tx}[,vnet_hdr_support] > > +@item -object > > +filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}, > > +queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id}][ > > +,insert=@var{after|before}] > > > > filter-mirror on netdev @var{netdevid},mirror net packet to > > chardev@var{chardevid}, if it has the vnet_hdr_support flag, filter-mirror will > > mirror packet with vnet_hdr_len. > > > > -@item -object filter- > > redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev= > > @var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support] > > +@item -object > > +filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevi > > +d},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,pos > > +ition=@var{head|tail|id}][,insert=@var{after|before}] > > > > filter-redirector on netdev @var{netdevid},redirect filter's net packet to > > chardev @var{chardevid},and redirect indev's packet to filter.if it has the > > vnet_hdr_support flag, @@ -4400,7 +4400,7 @@ Create a filter-redirector we > > need to differ outdev id from indev id, id can not be the same. we can just use > > indev or outdev, but at least one of indev or outdev need to be specified. > > > > -@item -object filter- > > rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_ > > support] > > +@item -object > > +filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx} > > +,[vnet_hdr_support][,position=@var{head|tail|id}][,insert=@var{after|be > > +fore}] > > > > Filter-rewriter is a part of COLO project.It will rewrite tcp packet to secondary > > from primary to keep secondary tcp connection,and rewrite @@ -4413,7 > > +4413,7 @@ colo secondary: > > -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 > > -object filter-rewriter,id=rew0,netdev=hn0,queue=all > > > > -@item -object filter- > > dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}] > > +@item -object > > +filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen= > > +@var{len}][,position=@var{head|tail|id}][,insert=@var{after|before}] > > > > Dump the network traffic on netdev @var{dev} to the file specified by > > @var{filename}. At most @var{len} bytes (64k by default) per packet are stored. > > -- > > 2.20.1 >
> -----Original Message----- > From: Lukas Straub <lukasstraub2@web.de> > Sent: Friday, August 23, 2019 2:21 PM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>; > Xie Changlong <xiechanglong.d@gmail.com> > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters > anywhere in the filter list > > On Fri, 23 Aug 2019 03:24:02 +0000 > "Zhang, Chen" <chen.zhang@intel.com> wrote: > > > > -----Original Message----- > > > From: Lukas Straub [mailto:lukasstraub2@web.de] > > > Sent: Friday, August 16, 2019 2:49 AM > > > To: qemu-devel <qemu-devel@nongnu.org> > > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > > > <jasowang@redhat.com>; Wen Congyang > <wencongyang2@huawei.com>; Xie > > > Changlong <xiechanglong.d@gmail.com> > > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters > > > anywhere in the filter list > > > > > > To switch the Secondary to Primary, we need to insert new filters > > > before the filter-rewriter. > > > > > > Add the options insert= and position= to be able to insert filters > > > anywhere in the filter list. > > > > > > position should be either "head", "tail" or the id of another filter. > > > insert should be either "before" or "after" to specify where to > > > insert the new filter relative to the one specified with position. > > > > > > > Hi Lukas, > > > > It looks no need to add the "insert = xxx" for this operation. > > For example: > > > > We have 3 net-filters, the running order like that: > > > > Fiter1 ----------> Filter2 ------------> Filter3 > > > > If we want to add another filter between filter1 and filter2. > > The "Position = head, insert = after" always seam with "position = filter2 id, > insert = before". > > Hi Zhang, > The insert= parameter is ignored if position=head or tail. It always Inserts at > the head (before Filter1) or the tail (after Filter3) of the List in these cases. > > > It seems the "insert" is a redundant args. > > So I think it is enough with the "position", we can make the "insert" always > equal "after" except the "head". > > Yes, we still could do it without it, but its more convenient with the insert= > parameter. For example our Case with inserting before the rewriter: > > 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': > 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': > { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': > 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', > 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } > > You see directly that here 3 Filters are inserted before the rewriter. > > would have to become: > > 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx', > 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0', > 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id': > 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': > 'compare0' } > > Which is less obvious. OK, It is fine for me. But in the code have some other issues like that: + +static void netfilter_set_insert(Object *obj, const char *str, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (strcmp(str, "before") && strcmp(str, "after")) { + error_setg(errp, "Invalid value for netfilter insert, " + "should be 'head' or 'tail'"); -------------------------------->>> I think you should change the "head/tail" to "before/after". + return; + } + + nf->insert_before = !strcmp(str, "before"); +} And I think the "front/behind" is better than "before/after" in this status. At the same time the name of the "insert_before" change to "front_flag" is better. Thanks Zhang Chen > > Regards, > Lukas Straub > > > > > Thanks > > Zhang Chen > > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > > --- > > > include/net/filter.h | 2 ++ > > > net/filter.c | 71 > +++++++++++++++++++++++++++++++++++++++++++- > > > qemu-options.hx | 10 +++---- > > > 3 files changed, 77 insertions(+), 6 deletions(-)
On Mon, 2 Sep 2019 11:43:57 +0000 "Zhang, Chen" <chen.zhang@intel.com> wrote: > > -----Original Message----- > > From: Lukas Straub <lukasstraub2@web.de> > > Sent: Friday, August 23, 2019 2:21 PM > > To: Zhang, Chen <chen.zhang@intel.com> > > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang > > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>; > > Xie Changlong <xiechanglong.d@gmail.com> > > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters > > anywhere in the filter list > > > > On Fri, 23 Aug 2019 03:24:02 +0000 > > "Zhang, Chen" <chen.zhang@intel.com> wrote: > > > > > > -----Original Message----- > > > > From: Lukas Straub [mailto:lukasstraub2@web.de] > > > > Sent: Friday, August 16, 2019 2:49 AM > > > > To: qemu-devel <qemu-devel@nongnu.org> > > > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > > > > <jasowang@redhat.com>; Wen Congyang > > <wencongyang2@huawei.com>; Xie > > > > Changlong <xiechanglong.d@gmail.com> > > > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters > > > > anywhere in the filter list > > > > > > > > To switch the Secondary to Primary, we need to insert new filters > > > > before the filter-rewriter. > > > > > > > > Add the options insert= and position= to be able to insert filters > > > > anywhere in the filter list. > > > > > > > > position should be either "head", "tail" or the id of another filter. > > > > insert should be either "before" or "after" to specify where to > > > > insert the new filter relative to the one specified with position. > > > > > > > > > > Hi Lukas, > > > > > > It looks no need to add the "insert = xxx" for this operation. > > > For example: > > > > > > We have 3 net-filters, the running order like that: > > > > > > Fiter1 ----------> Filter2 ------------> Filter3 > > > > > > If we want to add another filter between filter1 and filter2. > > > The "Position = head, insert = after" always seam with "position = filter2 id, > > insert = before". > > > > Hi Zhang, > > The insert= parameter is ignored if position=head or tail. It always Inserts at > > the head (before Filter1) or the tail (after Filter3) of the List in these cases. > > > > > It seems the "insert" is a redundant args. > > > So I think it is enough with the "position", we can make the "insert" always > > equal "after" except the "head". > > > > Yes, we still could do it without it, but its more convenient with the insert= > > parameter. For example our Case with inserting before the rewriter: > > > > 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': > > 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': > > { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': > > 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', > > 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } > > > > You see directly that here 3 Filters are inserted before the rewriter. > > > > would have to become: > > > > 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx', > > 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0', > > 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id': > > 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': > > 'compare0' } > > > > Which is less obvious. > > OK, It is fine for me. > But in the code have some other issues like that: > > + > +static void netfilter_set_insert(Object *obj, const char *str, Error **errp) > +{ > + NetFilterState *nf = NETFILTER(obj); > + > + if (strcmp(str, "before") && strcmp(str, "after")) { > + error_setg(errp, "Invalid value for netfilter insert, " > + "should be 'head' or 'tail'"); -------------------------------->>> I think you should change the "head/tail" to "before/after". Oops, that was a typo. > + return; > + } > + > + nf->insert_before = !strcmp(str, "before"); > +} > > > And I think the "front/behind" is better than "before/after" in this status. > At the same time the name of the "insert_before" change to "front_flag" is better. What I like about the "before" is that it sounds more like an English sentence. insert: before, position: rew0 vs. insert: front, position: rew0 But I agree, "behind" is more clear than "after". What do you think about "first/last" instead of "head/tail"? Regards, Lukas Straub > > Thanks > Zhang Chen > > > > > Regards, > > Lukas Straub > > > > > > > > Thanks > > > Zhang Chen > > > > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > > > --- > > > > include/net/filter.h | 2 ++ > > > > net/filter.c | 71 > > +++++++++++++++++++++++++++++++++++++++++++- > > > > qemu-options.hx | 10 +++---- > > > > 3 files changed, 77 insertions(+), 6 deletions(-) >
> -----Original Message----- > From: Lukas Straub <lukasstraub2@web.de> > Sent: Tuesday, September 3, 2019 2:51 AM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>; > Xie Changlong <xiechanglong.d@gmail.com> > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters > anywhere in the filter list > > On Mon, 2 Sep 2019 11:43:57 +0000 > "Zhang, Chen" <chen.zhang@intel.com> wrote: > > > > -----Original Message----- > > > From: Lukas Straub <lukasstraub2@web.de> > > > Sent: Friday, August 23, 2019 2:21 PM > > > To: Zhang, Chen <chen.zhang@intel.com> > > > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang > > > <jasowang@redhat.com>; Wen Congyang > <wencongyang2@huawei.com>; Xie > > > Changlong <xiechanglong.d@gmail.com> > > > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert > > > filters anywhere in the filter list > > > > > > On Fri, 23 Aug 2019 03:24:02 +0000 > > > "Zhang, Chen" <chen.zhang@intel.com> wrote: > > > > > > > > -----Original Message----- > > > > > From: Lukas Straub [mailto:lukasstraub2@web.de] > > > > > Sent: Friday, August 16, 2019 2:49 AM > > > > > To: qemu-devel <qemu-devel@nongnu.org> > > > > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang > > > > > <jasowang@redhat.com>; Wen Congyang > > > <wencongyang2@huawei.com>; Xie > > > > > Changlong <xiechanglong.d@gmail.com> > > > > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert > > > > > filters anywhere in the filter list > > > > > > > > > > To switch the Secondary to Primary, we need to insert new > > > > > filters before the filter-rewriter. > > > > > > > > > > Add the options insert= and position= to be able to insert > > > > > filters anywhere in the filter list. > > > > > > > > > > position should be either "head", "tail" or the id of another filter. > > > > > insert should be either "before" or "after" to specify where to > > > > > insert the new filter relative to the one specified with position. > > > > > > > > > > > > > Hi Lukas, > > > > > > > > It looks no need to add the "insert = xxx" for this operation. > > > > For example: > > > > > > > > We have 3 net-filters, the running order like that: > > > > > > > > Fiter1 ----------> Filter2 ------------> Filter3 > > > > > > > > If we want to add another filter between filter1 and filter2. > > > > The "Position = head, insert = after" always seam with "position = > > > > filter2 id, > > > insert = before". > > > > > > Hi Zhang, > > > The insert= parameter is ignored if position=head or tail. It always > > > Inserts at the head (before Filter1) or the tail (after Filter3) of the List in > these cases. > > > > > > > It seems the "insert" is a redundant args. > > > > So I think it is enough with the "position", we can make the > > > > "insert" always > > > equal "after" except the "head". > > > > > > Yes, we still could do it without it, but its more convenient with > > > the insert= parameter. For example our Case with inserting before the > rewriter: > > > > > > 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', > 'netdev': > > > 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', > 'props': > > > { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': > > > 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { > > > 'insert': 'before', > > > 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': > > > 'compare0' } > > > > > > You see directly that here 3 Filters are inserted before the rewriter. > > > > > > would have to become: > > > > > > 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', > > > 'netdev': 'hn0', 'queue': 'tx', > > > 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': > > > { 'position': 'm0', > > > 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id': > > > 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', > 'outdev': > > > 'compare0' } > > > > > > Which is less obvious. > > > > OK, It is fine for me. > > But in the code have some other issues like that: > > > > + > > +static void netfilter_set_insert(Object *obj, const char *str, Error > > +**errp) { > > + NetFilterState *nf = NETFILTER(obj); > > + > > + if (strcmp(str, "before") && strcmp(str, "after")) { > > + error_setg(errp, "Invalid value for netfilter insert, " > > + "should be 'head' or 'tail'"); ---------------------- > ---------->>> I think you should change the "head/tail" to "before/after". > > Oops, that was a typo. > > > + return; > > + } > > + > > + nf->insert_before = !strcmp(str, "before"); } > > > > > > And I think the "front/behind" is better than "before/after" in this status. > > At the same time the name of the "insert_before" change to "front_flag" is > better. > > What I like about the "before" is that it sounds more like an English sentence. > insert: before, position: rew0 > vs. > insert: front, position: rew0 > > But I agree, "behind" is more clear than "after". > > What do you think about "first/last" instead of "head/tail"? I'm not a English native speaker, but I think head/tail is enough same as the QTAILQ. Thanks Zhang Chen > > Regards, > Lukas Straub > > > > > Thanks > > Zhang Chen > > > > > > > > Regards, > > > Lukas Straub > > > > > > > > > > > Thanks > > > > Zhang Chen > > > > > > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > > > > --- > > > > > include/net/filter.h | 2 ++ > > > > > net/filter.c | 71 > > > +++++++++++++++++++++++++++++++++++++++++++- > > > > > qemu-options.hx | 10 +++---- > > > > > 3 files changed, 77 insertions(+), 6 deletions(-) > >
diff --git a/include/net/filter.h b/include/net/filter.h index 49da666ac0..355c178f75 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -62,6 +62,8 @@ struct NetFilterState { NetClientState *netdev; NetFilterDirection direction; bool on; + char *position; + bool insert_before; QTAILQ_ENTRY(NetFilterState) next; }; diff --git a/net/filter.c b/net/filter.c index 28d1930db7..309fd778df 100644 --- a/net/filter.c +++ b/net/filter.c @@ -171,11 +171,47 @@ static void netfilter_set_status(Object *obj, const char *str, Error **errp) } } +static char *netfilter_get_position(Object *obj, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + return g_strdup(nf->position); +} + +static void netfilter_set_position(Object *obj, const char *str, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + nf->position = g_strdup(str); +} + +static char *netfilter_get_insert(Object *obj, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + return nf->insert_before ? g_strdup("before") : g_strdup("after"); +} + +static void netfilter_set_insert(Object *obj, const char *str, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (strcmp(str, "before") && strcmp(str, "after")) { + error_setg(errp, "Invalid value for netfilter insert, " + "should be 'head' or 'tail'"); + return; + } + + nf->insert_before = !strcmp(str, "before"); +} + static void netfilter_init(Object *obj) { NetFilterState *nf = NETFILTER(obj); nf->on = true; + nf->insert_before = false; + nf->position = g_strdup("tail"); object_property_add_str(obj, "netdev", netfilter_get_netdev_id, netfilter_set_netdev_id, @@ -187,11 +223,18 @@ static void netfilter_init(Object *obj) object_property_add_str(obj, "status", netfilter_get_status, netfilter_set_status, NULL); + object_property_add_str(obj, "position", + netfilter_get_position, netfilter_set_position, + NULL); + object_property_add_str(obj, "insert", + netfilter_get_insert, netfilter_set_insert, + NULL); } static void netfilter_complete(UserCreatable *uc, Error **errp) { NetFilterState *nf = NETFILTER(uc); + NetFilterState *position = NULL; NetClientState *ncs[MAX_QUEUE_NUM]; NetFilterClass *nfc = NETFILTER_GET_CLASS(uc); int queues; @@ -219,6 +262,20 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) return; } + if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) { + /* Search for the position to insert before/after */ + Object *container; + Object *obj; + + container = object_get_objects_root(); + obj = object_resolve_path_component(container, nf->position); + if (!obj) { + error_setg(errp, "filter '%s' not found", nf->position); + return; + } + position = NETFILTER(obj); + } + nf->netdev = ncs[0]; if (nfc->setup) { @@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) return; } } - QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); + + if (position) { + if (nf->insert_before) { + QTAILQ_INSERT_BEFORE(position, nf, next); + } else { + QTAILQ_INSERT_AFTER(&nf->netdev->filters, position, nf, next); + } + } else if (!strcmp(nf->position, "head")) { + QTAILQ_INSERT_HEAD(&nf->netdev->filters, nf, next); + } else if (!strcmp(nf->position, "tail")) { + QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); + } } static void netfilter_finalize(Object *obj) @@ -245,6 +313,7 @@ static void netfilter_finalize(Object *obj) QTAILQ_REMOVE(&nf->netdev->filters, nf, next); } g_free(nf->netdev_id); + g_free(nf->position); } static void default_handle_event(NetFilterState *nf, int event, Error **errp) diff --git a/qemu-options.hx b/qemu-options.hx index 08749a3391..f0a47a0746 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4368,7 +4368,7 @@ applications, they can do this through this parameter. Its format is a gnutls priority string as described at @url{https://gnutls.org/manual/html_node/Priority-Strings.html}. -@item -object filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|rx|tx}][,status=@var{on|off}] +@item -object filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|rx|tx}][,status=@var{on|off}][,position=@var{head|tail|id}][,insert=@var{after|before}] Interval @var{t} can't be 0, this filter batches the packet delivery: all packets arriving in a given interval on netdev @var{netdevid} are delayed @@ -4387,11 +4387,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter. @option{tx}: the filter is attached to the transmit queue of the netdev, where it will receive packets sent by the netdev. -@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support] +@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id}][,insert=@var{after|before}] filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if it has the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len. -@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support] +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id}][,insert=@var{after|before}] filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev @var{chardevid},and redirect indev's packet to filter.if it has the vnet_hdr_support flag, @@ -4400,7 +4400,7 @@ Create a filter-redirector we need to differ outdev id from indev id, id can not be the same. we can just use indev or outdev, but at least one of indev or outdev need to be specified. -@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_support] +@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_support][,position=@var{head|tail|id}][,insert=@var{after|before}] Filter-rewriter is a part of COLO project.It will rewrite tcp packet to secondary from primary to keep secondary tcp connection,and rewrite @@ -4413,7 +4413,7 @@ colo secondary: -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 -object filter-rewriter,id=rew0,netdev=hn0,queue=all -@item -object filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}] +@item -object filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}][,position=@var{head|tail|id}][,insert=@var{after|before}] Dump the network traffic on netdev @var{dev} to the file specified by @var{filename}. At most @var{len} bytes (64k by default) per packet are stored.
To switch the Secondary to Primary, we need to insert new filters before the filter-rewriter. Add the options insert= and position= to be able to insert filters anywhere in the filter list. position should be either "head", "tail" or the id of another filter. insert should be either "before" or "after" to specify where to insert the new filter relative to the one specified with position. Signed-off-by: Lukas Straub <lukasstraub2@web.de> --- include/net/filter.h | 2 ++ net/filter.c | 71 +++++++++++++++++++++++++++++++++++++++++++- qemu-options.hx | 10 +++---- 3 files changed, 77 insertions(+), 6 deletions(-) -- 2.20.1