[ovs-dev,2/2] Revert "dpctl: Expand the flow dump type filter"

Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a
number of issues with style, build breakage, and failing unit tests.
The patch is being reverted so that they can addressed.

This reverts commit ab15e70eb5878b46f8f84da940ffc915b6d74cad.

CC: Gavi Teitz <gavi@mellanox.com>
CC: Simon Horman <simon.horman@netronome.com>
CC: Roi Dayan <roid@mellanox.com>
CC: Aaron Conole <aconole@redhat.com>
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 lib/dpctl.c         | 107 +++++++++++---------------------------------
 lib/dpctl.man       |  14 ++----
 lib/dpif-netlink.c  |  44 +++++++++++-------
 lib/dpif-provider.h |   5 +--
 lib/dpif.c          |   5 +--
 lib/dpif.h          |   7 +--
 6 files changed, 63 insertions(+), 119 deletions(-)

On Wed, Jul 25, 2018 at 02:01:32PM -0700, Justin Pettit wrote:
> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a
> number of issues with style, build breakage, and failing unit tests.
> The patch is being reverted so that they can addressed.
> 
> This reverts commit ab15e70eb5878b46f8f84da940ffc915b6d74cad.
> 
> CC: Gavi Teitz <gavi@mellanox.com>
> CC: Simon Horman <simon.horman@netronome.com>
> CC: Roi Dayan <roid@mellanox.com>
> CC: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

For the series:
        Acked-by: Ben Pfaff <blp@ovn.org>
since it breaks unit tests and it isn't obvious how to fix them.

But I suggest giving Simon and the others a chance to respond.

By the way, when you have CC: in a commit message, you're supposed to
actually CC those people in the email.  "git send-email" does that
automatically for me but it looks like it didn't pick them up for you.

> On Jul 25, 2018, at 2:11 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Wed, Jul 25, 2018 at 02:01:32PM -0700, Justin Pettit wrote:
>> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a
>> number of issues with style, build breakage, and failing unit tests.
>> The patch is being reverted so that they can addressed.
>> 
>> This reverts commit ab15e70eb5878b46f8f84da940ffc915b6d74cad.
>> 
>> CC: Gavi Teitz <gavi@mellanox.com>
>> CC: Simon Horman <simon.horman@netronome.com>
>> CC: Roi Dayan <roid@mellanox.com>
>> CC: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> 
> For the series:
>        Acked-by: Ben Pfaff <blp@ovn.org>
> since it breaks unit tests and it isn't obvious how to fix them.

Thanks.

> But I suggest giving Simon and the others a chance to respond.

Okay.

> By the way, when you have CC: in a commit message, you're supposed to
> actually CC those people in the email.  "git send-email" does that
> automatically for me but it looks like it didn't pick them up for you.

Hmm.  My "git send-email" says that they were all cc'd in the output of the command, but my mail client only shows Simon.  I don't know where, or if, there's a problem, but it thinks it worked.

--Justin

Hi,

On 25 July 2018 at 23:17, Justin Pettit <jpettit@ovn.org> wrote:

>
> > On Jul 25, 2018, at 2:11 PM, Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Wed, Jul 25, 2018 at 02:01:32PM -0700, Justin Pettit wrote:
> >> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a
> >> number of issues with style, build breakage, and failing unit tests.
> >> The patch is being reverted so that they can addressed.
> >>
> >> This reverts commit ab15e70eb5878b46f8f84da940ffc915b6d74cad.
> >>
> >> CC: Gavi Teitz <gavi@mellanox.com>
> >> CC: Simon Horman <simon.horman@netronome.com>
> >> CC: Roi Dayan <roid@mellanox.com>
> >> CC: Aaron Conole <aconole@redhat.com>
> >> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> >
> > For the series:
> >        Acked-by: Ben Pfaff <blp@ovn.org>
> > since it breaks unit tests and it isn't obvious how to fix them.
>
> Thanks.
>
> > But I suggest giving Simon and the others a chance to respond.
>
> Okay.
>
> > By the way, when you have CC: in a commit message, you're supposed to
> > actually CC those people in the email.  "git send-email" does that
> > automatically for me but it looks like it didn't pick them up for you.
>
> Hmm.  My "git send-email" says that they were all cc'd in the output of
> the command, but my mail client only shows Simon.  I don't know where, or
> if, there's a problem, but it thinks it worked.


Sorry about this. I agree a revert was the correct course of action.

> On Jul 25, 2018, at 11:29 PM, Simon Horman <simon.horman@netronome.com> wrote:
> 
> Sorry about this. I agree a revert was the correct course of action. 

Thanks.  I pushed the series to master.

--Justin

From: Justin Pettit, sent: Thursday, July 26, 2018 12:02 AM:
> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a number of issues with style, build breakage, and failing unit tests.
> The patch is being reverted so that they can addressed.

I acknowledge the build breakage issue, could you elaborate regarding the style issues?

As for the failing unit tests, this commit provides the means to fix unit tests that lost their relevance due to the changes introduced in commit d63ca5329ff9 ("dpctl: Properly reflect a rule's offloaded to HW state"). Are there other unit tests that are broken due to this commit?

Thanks!

-----Original Message-----
From: Justin Pettit [mailto:jpettit@ovn.org] 
Sent: Thursday, July 26, 2018 12:02 AM
To: dev@openvswitch.org
Cc: Gavi Teitz <gavi@mellanox.com>; Simon Horman <simon.horman@netronome.com>; Roi Dayan <roid@mellanox.com>; Aaron Conole <aconole@redhat.com>
Subject: [PATCH 2/2] Revert "dpctl: Expand the flow dump type filter"

Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a number of issues with style, build breakage, and failing unit tests.
The patch is being reverted so that they can addressed.

This reverts commit ab15e70eb5878b46f8f84da940ffc915b6d74cad.

CC: Gavi Teitz <gavi@mellanox.com>
CC: Simon Horman <simon.horman@netronome.com>
CC: Roi Dayan <roid@mellanox.com>
CC: Aaron Conole <aconole@redhat.com>
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 lib/dpctl.c         | 107 +++++++++++---------------------------------
 lib/dpctl.man       |  14 ++----
 lib/dpif-netlink.c  |  44 +++++++++++-------
 lib/dpif-provider.h |   5 +--
 lib/dpif.c          |   5 +--
 lib/dpif.h          |   7 +--
 6 files changed, 63 insertions(+), 119 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 9e504c9d1fff..4f1e443f2662 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -792,80 +792,18 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports,
     format_odp_actions(ds, f->actions, f->actions_len, ports);  }
 
-struct dump_types {
-    bool ovs;
-    bool tc;
-    bool offloaded;
-    bool non_offloaded;
+static char *supported_dump_types[] = {
+    "offloaded",
+    "ovs",
 };
 
-static void
-enable_all_dump_types(struct dump_types *dump_types) -{
-    dump_types->ovs = true;
-    dump_types->tc = true;
-    dump_types->offloaded = true;
-    dump_types->non_offloaded = true;
-}
-
-static int
-populate_dump_types(char *types_list, struct dump_types *dump_types,
-                    struct dpctl_params *dpctl_p)
-{
-    if (!types_list) {
-        enable_all_dump_types(dump_types);
-        return 0;
-    }
-
-    char *current_type;
-    while (types_list && types_list[0] != '\0') {
-        current_type = types_list;
-        size_t type_len = strcspn(current_type, ",");
-        types_list += type_len + (types_list[type_len] != '\0');
-        current_type[type_len] = '\0';
-
-        if (!strcmp(current_type, "ovs")) {
-            dump_types->ovs = true;
-        } else if (!strcmp(current_type, "tc")) {
-            dump_types->tc = true;
-        } else if (!strcmp(current_type, "offloaded")) {
-            dump_types->offloaded = true;
-        } else if (!strcmp(current_type, "non-offloaded")) {
-            dump_types->non_offloaded = true;
-        } else if (!strcmp(current_type, "all")) {
-            enable_all_dump_types(dump_types);
-        } else {
-            dpctl_error(dpctl_p, EINVAL, "Failed to parse type (%s)", current_type);
-            return EINVAL;
-        }
-    }
-    return 0;
-}
-
-static void
-determine_dpif_flow_dump_types(struct dump_types *dump_types,
-                               struct dpif_flow_dump_types *dpif_dump_types) {
-    dpif_dump_types->ovs_flows = dump_types->ovs || dump_types->non_offloaded;
-    dpif_dump_types->netdev_flows = dump_types->tc || dump_types->offloaded
-                                    || dump_types->non_offloaded;
-}
-
 static bool
-flow_passes_type_filter(const struct dpif_flow *f, struct dump_types *dump_types)
+flow_passes_type_filter(const struct dpif_flow *f, char *type)
 {
-    if (dump_types->ovs && !strcmp(f->attrs.dp_layer, "ovs")) {
-        return true;
-    }
-    if (dump_types->tc && !strcmp(f->attrs.dp_layer, "tc")) {
-        return true;
-    }
-    if (dump_types->offloaded && f->attrs.offloaded) {
-        return true;
-    }
-    if (dump_types->non_offloaded && !(f->attrs.offloaded)) {
-        return true;
+    if (!strcmp(type, "offloaded")) {
+        return f->attrs.offloaded;
     }
-    return false;
+    return true;
 }
 
 static struct hmap *
@@ -905,11 +843,9 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
     struct ds ds;
 
     char *filter = NULL;
+    char *type = NULL;
     struct flow flow_filter;
     struct flow_wildcards wc_filter;
-    char *types_list = NULL;
-    struct dump_types dump_types;
-    struct dpif_flow_dump_types dpif_dump_types;
 
     struct dpif_flow_dump_thread *flow_dump_thread;
     struct dpif_flow_dump *flow_dump;
@@ -922,8 +858,8 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
         lastargc = argc;
         if (!strncmp(argv[argc - 1], "filter=", 7) && !filter) {
             filter = xstrdup(argv[--argc] + 7);
-        } else if (!strncmp(argv[argc - 1], "type=", 5) && !types_list) {
-            types_list = xstrdup(argv[--argc] + 5);
+        } else if (!strncmp(argv[argc - 1], "type=", 5) && !type) {
+            type = xstrdup(argv[--argc] + 5);
         }
     }
 
@@ -956,12 +892,19 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
         }
     }
 
-    memset(&dump_types, 0, sizeof dump_types);
-    error = populate_dump_types(types_list, &dump_types, dpctl_p);
-    if (error) {
-        goto out_free;
+    if (type) {
+        error = EINVAL;
+        for (int i = 0; i < ARRAY_SIZE(supported_dump_types); i++) {
+            if (!strcmp(supported_dump_types[i], type)) {
+                error = 0;
+                break;
+            }
+        }
+        if (error) {
+            dpctl_error(dpctl_p, error, "Failed to parse type (%s)", type);
+            goto out_free;
+        }
     }
-    determine_dpif_flow_dump_types(&dump_types, &dpif_dump_types);
 
     /* Make sure that these values are different. PMD_ID_NULL means that the
      * pmd is unspecified (e.g. because the datapath doesn't have different @@ -971,7 +914,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
 
     ds_init(&ds);
     memset(&f, 0, sizeof f);
-    flow_dump = dpif_flow_dump_create(dpif, false, &dpif_dump_types);
+    flow_dump = dpif_flow_dump_create(dpif, false, (type ? type : 
+ "dpctl"));
     flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
     while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
         if (filter) {
@@ -1007,7 +950,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
             }
             pmd_id = f.pmd_id;
         }
-        if (flow_passes_type_filter(&f, &dump_types)) {
+        if (!type || flow_passes_type_filter(&f, type)) {
             format_dpif_flow(&ds, &f, portno_names, dpctl_p);
             dpctl_print(dpctl_p, "%s\n", ds_cstr(&ds));
         }
@@ -1025,7 +968,7 @@ out_dpifclose:
     dpif_close(dpif);
 out_free:
     free(filter);
-    free(types_list);
+    free(type);
     return error;
 }
 
diff --git a/lib/dpctl.man b/lib/dpctl.man index b77bc3fd8ad6..5d987e62daaa 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -118,16 +118,10 @@ The \fIfilter\fR is also useful to match wildcarded fields in the datapath  flow. As an example, \fBfilter='tcp,tp_src=100'\fR will match the  datapath flow containing '\fBtcp(src=80/0xff00,dst=8080/0xff)\fR'.
 .IP
-If \fBtype=\fItype\fR is specified, only displays flows of the specified type(s).
-\fItype\fR is a comma separated list, which can contain any of the following:
-.
-   \fBovs\fR - displays flows handled in the ovs dp
-   \fBtc\fR - displays flows handled in the tc dp
-   \fBoffloaded\fR - displays flows offloaded to the HW
-   \fBnon-offloaded\fR - displays flows not offloaded to the HW
-   \fBall\fR - displays all the types of flows
-.IP
-By default all the types of flows are displayed.
+If \fBtype=\fItype\fR is specified, only displays flows of a specific type.
+\fItype\fR can be \fBoffloaded\fR to display only rules offloaded to 
+the HW or \fBovs\fR to display only rules from the OVS tables.
+By default all rules are displayed.
 .
 .IP "\*(DX\fBadd\-flow\fR [\fIdp\fR] \fIflow actions\fR"
 .TP
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index e0edd9c2ffb1..62c7120e8f54 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1462,6 +1462,16 @@ dpif_netlink_init_flow_del(struct dpif_netlink *dpif,
                                  del->ufid, del->terse, request);  }
 
+enum {
+    DUMP_OVS_FLOWS_BIT    = 0,
+    DUMP_NETDEV_FLOWS_BIT = 1,
+};
+
+enum {
+    DUMP_OVS_FLOWS    = (1 << DUMP_OVS_FLOWS_BIT),
+    DUMP_NETDEV_FLOWS = (1 << DUMP_NETDEV_FLOWS_BIT), };
+
 struct dpif_netlink_flow_dump {
     struct dpif_flow_dump up;
     struct nl_dump nl_dump;
@@ -1470,7 +1480,7 @@ struct dpif_netlink_flow_dump {
     int netdev_dumps_num;                    /* Number of netdev_flow_dumps */
     struct ovs_mutex netdev_lock;            /* Guards the following. */
     int netdev_current_dump OVS_GUARDED;     /* Shared current dump */
-    struct dpif_flow_dump_types types;       /* Type of dump */
+    int type;                                /* Type of dump */
 };
 
 static struct dpif_netlink_flow_dump *
@@ -1485,7 +1495,7 @@ start_netdev_dump(const struct dpif *dpif_,  {
     ovs_mutex_init(&dump->netdev_lock);
 
-    if (!(dump->types.netdev_flows)) {
+    if (!(dump->type & DUMP_NETDEV_FLOWS)) {
         dump->netdev_dumps_num = 0;
         dump->netdev_dumps = NULL;
         return;
@@ -1499,20 +1509,24 @@ start_netdev_dump(const struct dpif *dpif_,
     ovs_mutex_unlock(&dump->netdev_lock);
 }
 
-static void
-dpif_netlink_populate_flow_dump_types(struct dpif_netlink_flow_dump *dump,
-                                      struct dpif_flow_dump_types *types) {
-    if (!types) {
-        dump->types.ovs_flows = true;
-        dump->types.netdev_flows = true;
-    } else {
-        memcpy(&dump->types, types, sizeof *types);
+static int
+dpif_netlink_get_dump_type(char *str) {
+    int type = 0;
+
+    if (!str || !strcmp(str, "ovs") || !strcmp(str, "dpctl")) {
+        type |= DUMP_OVS_FLOWS;
     }
+    if ((netdev_is_flow_api_enabled() && !str)
+        || (str && (!strcmp(str, "offloaded") || !strcmp(str, "dpctl")))) {
+        type |= DUMP_NETDEV_FLOWS;
+    }
+
+    return type;
 }
 
 static struct dpif_flow_dump *
 dpif_netlink_flow_dump_create(const struct dpif *dpif_, bool terse,
-                              struct dpif_flow_dump_types *types)
+                              char *type)
 {
     const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
     struct dpif_netlink_flow_dump *dump; @@ -1522,9 +1536,9 @@ dpif_netlink_flow_dump_create(const struct dpif *dpif_, bool terse,
     dump = xmalloc(sizeof *dump);
     dpif_flow_dump_init(&dump->up, dpif_);
 
-    dpif_netlink_populate_flow_dump_types(dump, types);
+    dump->type = dpif_netlink_get_dump_type(type);
 
-    if (dump->types.ovs_flows) {
+    if (dump->type & DUMP_OVS_FLOWS) {
         dpif_netlink_flow_init(&request);
         request.cmd = OVS_FLOW_CMD_GET;
         request.dp_ifindex = dpif->dp_ifindex; @@ -1551,7 +1565,7 @@ dpif_netlink_flow_dump_destroy(struct dpif_flow_dump *dump_)
     unsigned int nl_status = 0;
     int dump_status;
 
-    if (dump->types.ovs_flows) {
+    if (dump->type & DUMP_OVS_FLOWS) {
         nl_status = nl_dump_done(&dump->nl_dump);
     }
 
@@ -1787,7 +1801,7 @@ dpif_netlink_flow_dump_next(struct dpif_flow_dump_thread *thread_,
         }
     }
 
-    if (!(dump->types.ovs_flows)) {
+    if (!(dump->type & DUMP_OVS_FLOWS)) {
         return n_flows;
     }
 
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 0d2075bf48a4..62b3598acfc5 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -284,10 +284,9 @@ struct dpif_class {
      * If 'terse' is true, then only UID and statistics will
      * be returned in the dump. Otherwise, all fields will be returned.
      *
-     * If 'types' isn't null, dumps only the flows of the passed types. */
+     * If 'type' isn't null, dumps only the flows of the given type. */
     struct dpif_flow_dump *(*flow_dump_create)(const struct dpif *dpif,
-                                               bool terse,
-                                               struct dpif_flow_dump_types *types);
+                                               bool terse, char *type);
     int (*flow_dump_destroy)(struct dpif_flow_dump *dump);
 
     struct dpif_flow_dump_thread *(*flow_dump_thread_create)( diff --git a/lib/dpif.c b/lib/dpif.c index 21bc062269aa..d78330bef3b8 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1080,10 +1080,9 @@ dpif_flow_del(struct dpif *dpif,
  * This function always successfully returns a dpif_flow_dump.  Error
  * reporting is deferred to dpif_flow_dump_destroy(). */  struct dpif_flow_dump * -dpif_flow_dump_create(const struct dpif *dpif, bool terse,
-                      struct dpif_flow_dump_types *types)
+dpif_flow_dump_create(const struct dpif *dpif, bool terse, char *type)
 {
-    return dpif->dpif_class->flow_dump_create(dpif, terse, types);
+    return dpif->dpif_class->flow_dump_create(dpif, terse, type);
 }
 
 /* Destroys 'dump', which must have been created with dpif_flow_dump_create().
diff --git a/lib/dpif.h b/lib/dpif.h
index 457cddc11f7e..33d2d0bec333 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -512,11 +512,6 @@ struct dpif_flow_attrs {
     const char *dp_layer;   /* DP layer the flow is handled in. */
 };
 
-struct dpif_flow_dump_types {
-    bool ovs_flows;
-    bool netdev_flows;
-};
-
 void dpif_flow_stats_extract(const struct flow *, const struct dp_packet *packet,
                              long long int used, struct dpif_flow_stats *);  void dpif_flow_stats_format(const struct dpif_flow_stats *, struct ds *); @@ -578,7 +573,7 @@ int dpif_flow_get(struct dpif *,
  * All error reporting is deferred to the call to dpif_flow_dump_destroy().
  */
 struct dpif_flow_dump *dpif_flow_dump_create(const struct dpif *, bool terse,
-                                             struct dpif_flow_dump_types *);
+                                             char *type);
 int dpif_flow_dump_destroy(struct dpif_flow_dump *);
 
 struct dpif_flow_dump_thread *dpif_flow_dump_thread_create(
--
2.17.1

> On Jul 26, 2018, at 7:29 AM, Gavi Teitz <gavi@mellanox.com> wrote:
> 
> From: Justin Pettit, sent: Thursday, July 26, 2018 12:02 AM:
>> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a number of issues with style, build breakage, and failing unit tests.
>> The patch is being reverted so that they can addressed.
> 
> I acknowledge the build breakage issue, could you elaborate regarding the style issues?

The main style issue was that lines shouldn't be over 79 characters longs.

> As for the failing unit tests, this commit provides the means to fix unit tests that lost their relevance due to the changes introduced in commit d63ca5329ff9 ("dpctl: Properly reflect a rule's offloaded to HW state"). Are there other unit tests that are broken due to this commit?

I saw about a dozen unit tests failing when I ran "make check".

Thanks,

--Justin

Hi Gavi,

On 26 July 2018 at 17:36, Justin Pettit <jpettit@ovn.org> wrote:

>
> > On Jul 26, 2018, at 7:29 AM, Gavi Teitz <gavi@mellanox.com> wrote:
> >
> > From: Justin Pettit, sent: Thursday, July 26, 2018 12:02 AM:
> >> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a
> number of issues with style, build breakage, and failing unit tests.
> >> The patch is being reverted so that they can addressed.
> >
> > I acknowledge the build breakage issue, could you elaborate regarding
> the style issues?
>
> The main style issue was that lines shouldn't be over 79 characters longs.
>
> > As for the failing unit tests, this commit provides the means to fix
> unit tests that lost their relevance due to the changes introduced in
> commit d63ca5329ff9 ("dpctl: Properly reflect a rule's offloaded to HW
> state"). Are there other unit tests that are broken due to this commit?
>
> I saw about a dozen unit tests failing when I ran "make check".
>

I believe this gives an overview of the failing tests:

https://travis-ci.org/horms2/ovs/jobs/408446254

On 27/07/2018 14:00, Simon Horman wrote:
> Hi Gavi,
> 
> On 26 July 2018 at 17:36, Justin Pettit <jpettit@ovn.org <mailto:jpettit@ovn.org>> wrote:
> 
> 
>     > On Jul 26, 2018, at 7:29 AM, Gavi Teitz <gavi@mellanox.com <mailto:gavi@mellanox.com>> wrote:
>     > 
>     > From: Justin Pettit, sent: Thursday, July 26, 2018 12:02 AM:
>     >> Commit ab15e70eb587 ("dpctl: Expand the flow dump type filter") had a number of issues with style, build breakage, and failing unit tests.
>     >> The patch is being reverted so that they can addressed.
>     > 
>     > I acknowledge the build breakage issue, could you elaborate regarding the style issues?
> 
>     The main style issue was that lines shouldn't be over 79 characters longs.
> 
>     > As for the failing unit tests, this commit provides the means to fix unit tests that lost their relevance due to the changes introduced in commit d63ca5329ff9 ("dpctl: Properly reflect a rule's offloaded to HW state"). Are there other unit tests that are broken due to this commit?
> 
>     I saw about a dozen unit tests failing when I ran "make check".
> 
> 
> I believe this gives an overview of the failing tests:
> 
> https://travis-ci.org/horms2/ovs/jobs/408446254 <https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fhorms2%2Fovs%2Fjobs%2F408446254&data=02%7C01%7Croid%40mellanox.com%7Cf5f8d6b35d8648f7c1fa08d5f3b04e7f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636682860947190671&sdata=hkRIPzuKSEzCR6lzchvbQKYL%2B%2F4kNukN5voglAE5QGw%3D&reserved=0>


sorry all about this. we missed it internally.
next time we'll make sure we do all needed testing before submitting.
we usually do submit to travis to run all internal checks.
we probably missed it with this patch.

as for what Gavi mean about the needing the commit to fix the tests
it's for the offload tests only.