[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PULL V2 3/6] hmp-commands: Add new HMP command for filter passthrou
From: |
Zhang, Chen |
Subject: |
RE: [PULL V2 3/6] hmp-commands: Add new HMP command for filter passthrough |
Date: |
Thu, 15 Jul 2021 03:07:46 +0000 |
> -----Original Message-----
> From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Sent: Thursday, July 15, 2021 2:48 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Daniel P. Berrangé <berrange@redhat.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>; Lukas
> Straub <lukasstraub2@web.de>
> Subject: Re: [PULL V2 3/6] hmp-commands: Add new HMP command for
> filter passthrough
>
> * Zhang Chen (chen.zhang@intel.com) wrote:
> > Add hmp_passthrough_filter_add and hmp_passthrough_filter_del make
> > user can maintain object network passthrough list in human monitor
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> > hmp-commands.hx | 26 +++++++++++++++++
> > include/monitor/hmp.h | 2 ++
> > monitor/hmp-cmds.c | 67
> +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 95 insertions(+)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx index
> > 8e45bce2cd..426a7d6cda 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1292,6 +1292,32 @@ SRST
> > Remove host network device.
> > ERST
> >
> > + {
> > + .name = "passthrough_filter_add",
> > + .args_type = "protocol:s?,object-name:s,src:s?,dst:s?",
> > + .params = "[protocol] object-name [src] [dst]",
> > + .help = "Add network passthrough rule to object passthrough
> > list",
> > + .cmd = hmp_passthrough_filter_add,
> > + },
> > +
> > +SRST
> > +``passthrough_filter_add``
> > + Add network stream to object passthrough list.
> > +ERST
> > +
> > + {
> > + .name = "passthrough_filter_del",
> > + .args_type = "protocol:s?,object-name:s,src:s?,dst:s?",
> > + .params = "[protocol] object-name [src] [dst]",
> > + .help = "Delete network passthrough rule from object
> > passthrough
> list",
> > + .cmd = hmp_passthrough_filter_del,
> > + },
> > +
> > +SRST
> > +``passthrough_filter_del``
> > + Delete network stream from object passthrough list.
> > +ERST
> > +
> > {
> > .name = "object_add",
> > .args_type = "object:S",
> > diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h index
> > 3baa1058e2..ba6987e552 100644
> > --- a/include/monitor/hmp.h
> > +++ b/include/monitor/hmp.h
> > @@ -77,6 +77,8 @@ void hmp_device_del(Monitor *mon, const QDict
> > *qdict); void hmp_dump_guest_memory(Monitor *mon, const QDict
> > *qdict); void hmp_netdev_add(Monitor *mon, const QDict *qdict); void
> > hmp_netdev_del(Monitor *mon, const QDict *qdict);
> > +void hmp_passthrough_filter_add(Monitor *mon, const QDict *qdict);
> > +void hmp_passthrough_filter_del(Monitor *mon, const QDict *qdict);
> > void hmp_getfd(Monitor *mon, const QDict *qdict); void
> > hmp_closefd(Monitor *mon, const QDict *qdict); void
> > hmp_sendkey(Monitor *mon, const QDict *qdict); diff --git
> > a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index
> 0942027208..b47a2e4850
> > 100644
> > --- a/monitor/hmp-cmds.c
> > +++ b/monitor/hmp-cmds.c
> > @@ -1638,6 +1638,73 @@ void hmp_netdev_del(Monitor *mon, const
> QDict *qdict)
> > hmp_handle_error(mon, err);
> > }
> >
> > +IPFlowSpec *hmp_parse_IPFlowSpec(Monitor *mon, const QDict *qdict)
> {
> > + IPFlowSpec *spec = g_new0(IPFlowSpec, 1);
> > + char *src, *dst;
> > +
> > + spec->protocol = g_strdup(qdict_get_try_str(qdict, "protocol"));
> > + spec->object_name = g_strdup(qdict_get_try_str(qdict, "object-
> name"));
> > + src = g_strdup(qdict_get_try_str(qdict, "src"));
> > + dst = g_strdup(qdict_get_try_str(qdict, "dst"));
> > +
> > + if (src) {
> > + spec->source = g_new0(InetSocketAddressBase, 1);
> > +
> > + if (inet_parse_base(spec->source, src, NULL)) {
> > + monitor_printf(mon, "Incorrect passthrough src address\n");
> > + g_free(spec->source);
> > + g_free(src);
> > + goto err;
>
> That leaks dst if dst is set.
>
> > + }
> > + g_free(src);
> > + }
> > +
> > + if (dst) {
> > + spec->destination = g_new0(InetSocketAddressBase, 1);
> > +
> > + if (inet_parse_base(spec->destination, dst, NULL)) {
> > + monitor_printf(mon, "Incorrect passthrough dst address\n");
> > + g_free(spec->destination);
> > + g_free(dst);
>
> That leaks src, and spec->source
>
> Perhaps the easiest thing would be:
> g_autofree *src = NULL;
> g_autofree *dst = NULL;
>
> and then they'll get free'd up automatically.
> Then in err: you can do a g_free() of spec->source and
> spec->destination and finally spec.
>
Oh, good idea. I will quick update it.
By the way, please review or tag other patches in this series.
Maybe we can catch up 6.1 merge window.
Thanks
Chen
> Dave
>
> > + goto err;
> > + }
> > + g_free(dst);
> > + }
> > +
> > + return spec;
> > +
> > +err:
> > + g_free(spec);
> > + return NULL;
> > +}
> > +
> > +void hmp_passthrough_filter_add(Monitor *mon, const QDict *qdict) {
> > + IPFlowSpec *spec;
> > + Error *err = NULL;
> > +
> > + spec = hmp_parse_IPFlowSpec(mon, qdict);
> > + if (spec) {
> > + qmp_passthrough_filter_add(spec, &err);
> > + }
> > +
> > + hmp_handle_error(mon, err);
> > +}
> > +
> > +void hmp_passthrough_filter_del(Monitor *mon, const QDict *qdict) {
> > + IPFlowSpec *spec;
> > + Error *err = NULL;
> > +
> > + spec = hmp_parse_IPFlowSpec(mon, qdict);
> > + if (spec) {
> > + qmp_passthrough_filter_del(spec, &err);
> > + }
> > +
> > + hmp_handle_error(mon, err);
> > +}
> > +
> > void hmp_object_add(Monitor *mon, const QDict *qdict) {
> > const char *options = qdict_get_str(qdict, "object");
> > --
> > 2.25.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
[PULL V2 4/6] net/colo-compare: Move data structure and define to .h file., Zhang Chen, 2021/07/01
[PULL V2 5/6] net/colo-compare: Add passthrough list to CompareState, Zhang Chen, 2021/07/01
[PULL V2 6/6] net/net.c: Add handler for passthrough filter command, Zhang Chen, 2021/07/01