diff mbox series

[PULL,V2,3/6] hmp-commands: Add new HMP command for filter passthrough

Message ID 20210701091130.3022093-4-chen.zhang@intel.com (mailing list archive)
State New, archived
Headers show
Series [PULL,V2,1/6] qapi/net: Add IPFlowSpec and QMP command for filter passthrough | expand

Commit Message

Zhang Chen July 1, 2021, 9:11 a.m. UTC
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(+)

Comments

Dr. David Alan Gilbert July 14, 2021, 6:47 p.m. UTC | #1
* 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.

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
>
Zhang Chen July 15, 2021, 3:07 a.m. UTC | #2
> -----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
Jason Wang July 15, 2021, 4:20 a.m. UTC | #3
在 2021/7/15 上午11:07, Zhang, Chen 写道:
> 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
>

Note that we probably miss the soft-freeze since it's actually a new 
feature.

It could only be done for 6.2.

Thanks
Zhang Chen July 19, 2021, 9 a.m. UTC | #4
> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, July 15, 2021 12:21 PM
> To: Zhang, Chen <chen.zhang@intel.com>; Dr. David Alan Gilbert
> <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: qemu-dev <qemu-devel@nongnu.org>; Eric Blake
> <eblake@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
> 
> 
> 在 2021/7/15 上午11:07, Zhang, Chen 写道:
> > 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
> >
> 
> Note that we probably miss the soft-freeze since it's actually a new feature.
> 
> It could only be done for 6.2.

OK, I will add for 6.2 tag in next pull.

Thanks
Chen

> 
> Thanks
diff mbox series

Patch

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;
+        }
+        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);
+            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");