diff mbox series

[for,7.0,V10,6/6] net/net.c: Add handler for passthrough filter command

Message ID 20211112031112.9303-7-chen.zhang@intel.com (mailing list archive)
State New, archived
Headers show
Series Add passthrough support to object with network processing function | expand

Commit Message

Zhang Chen Nov. 12, 2021, 3:11 a.m. UTC
Use the connection protocol,src port,dst port,src ip,dst ip as the key
to passthrough certain network traffic in object with network packet
processing function.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/net.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 197 insertions(+), 2 deletions(-)

Comments

Markus Armbruster Nov. 19, 2021, 4:02 p.m. UTC | #1
This is not review; I'm merely pointing out errors that caught my eye.

Zhang Chen <chen.zhang@intel.com> writes:

> Use the connection protocol,src port,dst port,src ip,dst ip as the key
> to passthrough certain network traffic in object with network packet
> processing function.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  net/net.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 197 insertions(+), 2 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 5d0d5914fb..443e88d396 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -55,6 +55,8 @@
>  #include "net/colo-compare.h"
>  #include "net/filter.h"
>  #include "qapi/string-output-visitor.h"
> +#include "net/colo-compare.h"
> +#include "qom/object_interfaces.h"
>  
>  /* Net bridge is currently not supported for W32. */
>  #if !defined(_WIN32)
> @@ -1215,14 +1217,207 @@ void qmp_netdev_del(const char *id, Error **errp)
>      }
>  }
>  
> +static int check_addr(InetSocketAddressBase *addr)
> +{
> +    if (!addr || (addr->host && !qemu_isdigit(addr->host[0]))) {
> +        return -1;
> +    }
> +
> +    if (atoi(addr->port) > 65536 || atoi(addr->port) < 0) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/* The initial version only supports colo-compare */
> +static CompareState *passthrough_filter_check(IPFlowSpec *spec, Error **errp)
> +{
> +    Object *container;
> +    Object *obj;
> +    CompareState *s;
> +
> +    if (!spec->object_name) {

How can spec->object_name ever be null?  It's not optional in the QAPI
schema.

> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "object-name",
> +                   "Need input object name");
> +        return NULL;
> +    }
> +
> +    container = object_get_objects_root();
> +    obj = object_resolve_path_component(container, spec->object_name);

As far as I can tell, object_resolve_path_component()'s second argument
is a property name, *not* a path.  I think you want

       s = (CompareState *)object_resolve_path_type(spec->object_name,
                                                    COLO_COMPARE, NULL);

This also takes care of ...

> +    if (!obj) {
> +        error_setg(errp, "object '%s' not found", spec->object_name);
> +        return NULL;
> +    }
> +
> +    s = COLO_COMPARE(obj);

... a probable bug here: when the object exists (@obj is not null), but
isn't a TYPE_COLO_COMPARE object, then @s is null here.  We can then
return null without setting an error.

> +
> +    if (!getprotobyname(spec->protocol)) {
> +        error_setg(errp, "Passthrough filter get wrong protocol");
> +        return NULL;
> +    }
> +
> +    if (spec->source) {
> +        if (check_addr(spec->source)) {
> +            error_setg(errp, "Passthrough filter get wrong source");
> +            return NULL;
> +        }
> +    }
> +
> +    if (spec->destination) {
> +        if (check_addr(spec->destination)) {
> +            error_setg(errp, "Passthrough filter get wrong destination");
> +            return NULL;
> +        }
> +    }
> +
> +    return s;
> +}
> +
> +/* The initial version only supports colo-compare */

Is there another version in the tree?  I guess not.  Recommend

   /* Supports only colo-compare so far */

Such limitations need to be clearly stated in the QAPI schema doc
comments.

> +static COLOPassthroughEntry *passthrough_filter_find(CompareState *s,
> +                                                     COLOPassthroughEntry *ent)
> +{
> +    COLOPassthroughEntry *next = NULL, *origin = NULL;
> +
> +    if (!QLIST_EMPTY(&s->passthroughlist)) {
> +        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
> +            if ((ent->l4_protocol.p_proto == origin->l4_protocol.p_proto) &&
> +                (ent->src_port == origin->src_port) &&
> +                (ent->dst_port == origin->dst_port) &&
> +                (ent->src_ip.s_addr == origin->src_ip.s_addr) &&
> +                (ent->dst_ip.s_addr == origin->dst_ip.s_addr)) {
> +                return origin;
> +            }
> +        }
> +    }
> +
> +    return NULL;
> +}

[...]
diff mbox series

Patch

diff --git a/net/net.c b/net/net.c
index 5d0d5914fb..443e88d396 100644
--- a/net/net.c
+++ b/net/net.c
@@ -55,6 +55,8 @@ 
 #include "net/colo-compare.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
+#include "net/colo-compare.h"
+#include "qom/object_interfaces.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -1215,14 +1217,207 @@  void qmp_netdev_del(const char *id, Error **errp)
     }
 }
 
+static int check_addr(InetSocketAddressBase *addr)
+{
+    if (!addr || (addr->host && !qemu_isdigit(addr->host[0]))) {
+        return -1;
+    }
+
+    if (atoi(addr->port) > 65536 || atoi(addr->port) < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+/* The initial version only supports colo-compare */
+static CompareState *passthrough_filter_check(IPFlowSpec *spec, Error **errp)
+{
+    Object *container;
+    Object *obj;
+    CompareState *s;
+
+    if (!spec->object_name) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "object-name",
+                   "Need input object name");
+        return NULL;
+    }
+
+    container = object_get_objects_root();
+    obj = object_resolve_path_component(container, spec->object_name);
+    if (!obj) {
+        error_setg(errp, "object '%s' not found", spec->object_name);
+        return NULL;
+    }
+
+    s = COLO_COMPARE(obj);
+
+    if (!getprotobyname(spec->protocol)) {
+        error_setg(errp, "Passthrough filter get wrong protocol");
+        return NULL;
+    }
+
+    if (spec->source) {
+        if (check_addr(spec->source)) {
+            error_setg(errp, "Passthrough filter get wrong source");
+            return NULL;
+        }
+    }
+
+    if (spec->destination) {
+        if (check_addr(spec->destination)) {
+            error_setg(errp, "Passthrough filter get wrong destination");
+            return NULL;
+        }
+    }
+
+    return s;
+}
+
+/* The initial version only supports colo-compare */
+static COLOPassthroughEntry *passthrough_filter_find(CompareState *s,
+                                                     COLOPassthroughEntry *ent)
+{
+    COLOPassthroughEntry *next = NULL, *origin = NULL;
+
+    if (!QLIST_EMPTY(&s->passthroughlist)) {
+        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
+            if ((ent->l4_protocol.p_proto == origin->l4_protocol.p_proto) &&
+                (ent->src_port == origin->src_port) &&
+                (ent->dst_port == origin->dst_port) &&
+                (ent->src_ip.s_addr == origin->src_ip.s_addr) &&
+                (ent->dst_ip.s_addr == origin->dst_ip.s_addr)) {
+                return origin;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+/* The initial version only supports colo-compare */
+static void passthrough_filter_add(CompareState *s,
+                                   IPFlowSpec *spec,
+                                   Error **errp)
+{
+    COLOPassthroughEntry *pass = NULL;
+
+    pass = g_new0(COLOPassthroughEntry, 1);
+
+    if (spec->protocol) {
+        memcpy(&pass->l4_protocol, getprotobyname(spec->protocol),
+               sizeof(struct protoent));
+    }
+
+    if (spec->source) {
+        if (!inet_aton(spec->source->host, &pass->src_ip)) {
+            pass->src_ip.s_addr = 0;
+        }
+
+        pass->src_port = atoi(spec->source->port);
+    }
+
+    if (spec->destination) {
+        if (!inet_aton(spec->destination->host, &pass->dst_ip)) {
+            pass->dst_ip.s_addr = 0;
+        }
+
+        pass->dst_port = atoi(spec->destination->port);
+    }
+
+    qemu_mutex_lock(&s->passthroughlist_mutex);
+    if (passthrough_filter_find(s, pass)) {
+        error_setg(errp, "The pass through connection already exists");
+        g_free(pass);
+        qemu_mutex_unlock(&s->passthroughlist_mutex);
+        return;
+    }
+
+    QLIST_INSERT_HEAD(&s->passthroughlist, pass, node);
+    qemu_mutex_unlock(&s->passthroughlist_mutex);
+}
+
+/* The initial version only supports colo-compare */
+static void passthrough_filter_del(CompareState *s,
+                                   IPFlowSpec *spec,
+                                   Error **errp)
+{
+    COLOPassthroughEntry *pass = NULL, *result = NULL;
+
+    pass = g_new0(COLOPassthroughEntry, 1);
+
+    if (spec->protocol) {
+        memcpy(&pass->l4_protocol, getprotobyname(spec->protocol),
+               sizeof(struct protoent));
+    }
+
+    if (spec->source) {
+        if (!inet_aton(spec->source->host, &pass->src_ip)) {
+            pass->src_ip.s_addr = 0;
+        }
+
+        pass->src_port = atoi(spec->source->port);
+    }
+
+    if (spec->destination) {
+        if (!inet_aton(spec->destination->host, &pass->dst_ip)) {
+            pass->dst_ip.s_addr = 0;
+        }
+
+        pass->dst_port = atoi(spec->destination->port);
+    }
+
+    qemu_mutex_lock(&s->passthroughlist_mutex);
+
+    result = passthrough_filter_find(s, pass);
+    if (result) {
+        QLIST_REMOVE(result, node);
+        g_free(result);
+    } else {
+        error_setg(errp, "Can't find the IP flow Spec");
+    }
+
+    g_free(pass);
+    g_free(spec);
+    qemu_mutex_unlock(&s->passthroughlist_mutex);
+}
+
+/* The initial version only supports colo-compare */
 void qmp_passthrough_filter_add(IPFlowSpec *spec, Error **errp)
 {
-    /* TODO implement setup passthrough rule */
+    CompareState *s;
+    Error *err = NULL;
+
+    s = passthrough_filter_check(spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    passthrough_filter_add(s, spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
 }
 
+/* The initial version only supports colo-compare */
 void qmp_passthrough_filter_del(IPFlowSpec *spec, Error **errp)
 {
-    /* TODO implement delete passthrough rule */
+    CompareState *s;
+    Error *err = NULL;
+
+    s = passthrough_filter_check(spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    passthrough_filter_del(s, spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
 }
 
 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)