diff mbox series

[ovs-dev,05/20] netdev-dpdk: Improve HW offload flow debuggability

Message ID 20191120152826.25074-6-elibr@mellanox.com
State Changes Requested
Delegated to: Ilya Maximets
Headers show
Series netdev datapath actions offload | expand

Commit Message

Eli Britstein Nov. 20, 2019, 3:28 p.m. UTC 
Add debug prints when creating and destroying rte flows.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
---
 lib/netdev-dpdk.c                 |  29 +++++++
 lib/netdev-offload-dpdk-flow.c    | 168 +++++++++++++++++++++++---------------
 lib/netdev-offload-dpdk-private.h |   5 ++
 3 files changed, 136 insertions(+), 66 deletions(-)

Comments

Li,Rongqing via dev Dec. 3, 2019, 3:03 p.m. UTC | #1 
On Wed, Nov 20, 2019 at 9:05 PM Eli Britstein <elibr@mellanox.com> wrote:
>
> Add debug prints when creating and destroying rte flows.
>
> Signed-off-by: Eli Britstein <elibr@mellanox.com>
> Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
> ---
>  lib/netdev-dpdk.c                 |  29 +++++++
>  lib/netdev-offload-dpdk-flow.c    | 168 +++++++++++++++++++++++---------------
>  lib/netdev-offload-dpdk-private.h |   5 ++
>  3 files changed, 136 insertions(+), 66 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 02120a379..673cbfbd6 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -67,6 +67,7 @@
>  #include "unixctl.h"
>  #include "util.h"
>  #include "uuid.h"
> +#include "netdev-offload-dpdk-private.h"
>
>  enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
>
> @@ -4452,6 +4453,15 @@ netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
>
>      ovs_mutex_lock(&dev->mutex);
>      ret = rte_flow_destroy(dev->port_id, rte_flow, error);
> +    if (!ret) {
> +        VLOG_DBG("Destroy rte_flow %p: netdev=%s, port=%d\n",
> +                 rte_flow, netdev_get_name(netdev), dev->port_id);
> +    } else {
> +        VLOG_ERR("Destroy rte_flow %p: netdev=%s, port=%d\n"
> +                 "FAILED. error %u : message : %s",
> +                 rte_flow, netdev_get_name(netdev), dev->port_id,
> +                 error->type, error->message);
> +    }

1. Ratelimiting macros (_RL) should be used for VLOG_DBG/ERR logs.
2. Can we move the logging code outside the lock ?

Thanks,
-Harsha
>      ovs_mutex_unlock(&dev->mutex);
>      return ret;
>  }
> @@ -4465,9 +4475,28 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
>  {
>      struct rte_flow *flow;
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +    struct ds s;
>
>      ovs_mutex_lock(&dev->mutex);
>      flow = rte_flow_create(dev->port_id, attr, items, actions, error);
> +    ds_init(&s);
> +    if (flow) {
> +        VLOG_DBG("Create rte_flow: netdev=%s, port=%d\n"
> +                 "%s"
> +                 "Flow handle=%p\n",
> +                 netdev_get_name(netdev), dev->port_id,
> +                 ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
> +                                                      actions)), flow);
> +    } else {
> +        VLOG_ERR("Create rte_flow: netdev=%s, port=%d\n"
> +                 "%s"
> +                 "FAILED. error %u : message : %s\n",
> +                 netdev_get_name(netdev), dev->port_id,
> +                 ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
> +                                                      actions)),
> +                 error->type, error->message);
> +    }
> +    ds_destroy(&s);
>      ovs_mutex_unlock(&dev->mutex);
>      return flow;
>  }
> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
> index c8c3e28ea..19c933932 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -61,73 +61,71 @@ netdev_dpdk_flow_actions_free(struct flow_actions *actions)
>  }
>
>  static void
> -dump_flow_pattern(struct rte_flow_item *item)
> +ds_put_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
>  {
> -    struct ds s;
> -
> -    if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
> -        return;
> -    }
> -
> -    ds_init(&s);
> +    ds_put_format(s,
> +                  "  Attributes: "
> +                  "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
> +                  attr->ingress, attr->egress, attr->priority, attr->group,
> +                  attr->transfer);
> +}
>
> +static void
> +ds_put_flow_pattern(struct ds *s, const struct rte_flow_item *item)
> +{
>      if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
>          const struct rte_flow_item_eth *eth_spec = item->spec;
>          const struct rte_flow_item_eth *eth_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow eth pattern:\n");
> +        ds_put_cstr(s, "rte flow eth pattern:\n");
>          if (eth_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
>                            "type=0x%04" PRIx16"\n",
>                            ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
>                            ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
>                            ntohs(eth_spec->type));
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (eth_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
>                            "type=0x%04"PRIx16"\n",
>                            ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
>                            ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
>                            ntohs(eth_mask->type));
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
>          const struct rte_flow_item_vlan *vlan_spec = item->spec;
>          const struct rte_flow_item_vlan *vlan_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow vlan pattern:\n");
> +        ds_put_cstr(s, "rte flow vlan pattern:\n");
>          if (vlan_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
>                            ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci));
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>
>          if (vlan_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
>                            ntohs(vlan_mask->inner_type), ntohs(vlan_mask->tci));
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
>          const struct rte_flow_item_ipv4 *ipv4_spec = item->spec;
>          const struct rte_flow_item_ipv4 *ipv4_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow ipv4 pattern:\n");
> +        ds_put_cstr(s, "rte flow ipv4 pattern:\n");
>          if (ipv4_spec) {
> -            ds_put_format(&s,
> -                          "  Spec: tos=0x%"PRIx8", ttl=%"PRIx8
> +            ds_put_format(s,
> +                          "  Spec: tos=0x%"PRIx8", ttl=%"PRIu8
>                            ", proto=0x%"PRIx8
>                            ", src="IP_FMT", dst="IP_FMT"\n",
>                            ipv4_spec->hdr.type_of_service,
> @@ -136,11 +134,11 @@ dump_flow_pattern(struct rte_flow_item *item)
>                            IP_ARGS(ipv4_spec->hdr.src_addr),
>                            IP_ARGS(ipv4_spec->hdr.dst_addr));
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (ipv4_mask) {
> -            ds_put_format(&s,
> -                          "  Mask: tos=0x%"PRIx8", ttl=%"PRIx8
> +            ds_put_format(s,
> +                          "  Mask: tos=0x%"PRIx8", ttl=%"PRIu8
>                            ", proto=0x%"PRIx8
>                            ", src="IP_FMT", dst="IP_FMT"\n",
>                            ipv4_mask->hdr.type_of_service,
> @@ -149,89 +147,81 @@ dump_flow_pattern(struct rte_flow_item *item)
>                            IP_ARGS(ipv4_mask->hdr.src_addr),
>                            IP_ARGS(ipv4_mask->hdr.dst_addr));
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
>          const struct rte_flow_item_udp *udp_spec = item->spec;
>          const struct rte_flow_item_udp *udp_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow udp pattern:\n");
> +        ds_put_cstr(s, "rte flow udp pattern:\n");
>          if (udp_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16"\n",
>                            ntohs(udp_spec->hdr.src_port),
>                            ntohs(udp_spec->hdr.dst_port));
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (udp_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: src_port=0x%"PRIx16
>                            ", dst_port=0x%"PRIx16"\n",
>                            ntohs(udp_mask->hdr.src_port),
>                            ntohs(udp_mask->hdr.dst_port));
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_SCTP) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_SCTP) {
>          const struct rte_flow_item_sctp *sctp_spec = item->spec;
>          const struct rte_flow_item_sctp *sctp_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow sctp pattern:\n");
> +        ds_put_cstr(s, "rte flow sctp pattern:\n");
>          if (sctp_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16"\n",
>                            ntohs(sctp_spec->hdr.src_port),
>                            ntohs(sctp_spec->hdr.dst_port));
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (sctp_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: src_port=0x%"PRIx16
>                            ", dst_port=0x%"PRIx16"\n",
>                            ntohs(sctp_mask->hdr.src_port),
>                            ntohs(sctp_mask->hdr.dst_port));
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_ICMP) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_ICMP) {
>          const struct rte_flow_item_icmp *icmp_spec = item->spec;
>          const struct rte_flow_item_icmp *icmp_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow icmp pattern:\n");
> +        ds_put_cstr(s, "rte flow icmp pattern:\n");
>          if (icmp_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: icmp_type=%"PRIu8", icmp_code=%"PRIu8"\n",
>                            icmp_spec->hdr.icmp_type,
>                            icmp_spec->hdr.icmp_code);
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (icmp_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: icmp_type=0x%"PRIx8
>                            ", icmp_code=0x%"PRIx8"\n",
>                            icmp_spec->hdr.icmp_type,
>                            icmp_spec->hdr.icmp_code);
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> -    }
> -
> -    if (item->type == RTE_FLOW_ITEM_TYPE_TCP) {
> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_TCP) {
>          const struct rte_flow_item_tcp *tcp_spec = item->spec;
>          const struct rte_flow_item_tcp *tcp_mask = item->mask;
>
> -        ds_put_cstr(&s, "rte flow tcp pattern:\n");
> +        ds_put_cstr(s, "rte flow tcp pattern:\n");
>          if (tcp_spec) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16
>                            ", data_off=0x%"PRIx8", tcp_flags=0x%"PRIx8"\n",
>                            ntohs(tcp_spec->hdr.src_port),
> @@ -239,10 +229,10 @@ dump_flow_pattern(struct rte_flow_item *item)
>                            tcp_spec->hdr.data_off,
>                            tcp_spec->hdr.tcp_flags);
>          } else {
> -            ds_put_cstr(&s, "  Spec = null\n");
> +            ds_put_cstr(s, "  Spec = null\n");
>          }
>          if (tcp_mask) {
> -            ds_put_format(&s,
> +            ds_put_format(s,
>                            "  Mask: src_port=%"PRIx16", dst_port=%"PRIx16
>                            ", data_off=0x%"PRIx8", tcp_flags=0x%"PRIx8"\n",
>                            ntohs(tcp_mask->hdr.src_port),
> @@ -250,12 +240,59 @@ dump_flow_pattern(struct rte_flow_item *item)
>                            tcp_mask->hdr.data_off,
>                            tcp_mask->hdr.tcp_flags);
>          } else {
> -            ds_put_cstr(&s, "  Mask = null\n");
> +            ds_put_cstr(s, "  Mask = null\n");
>          }
> +    } else {
> +        ds_put_format(s, "unknown rte flow pattern (%d)\n", item->type);
> +    }
> +}
> +
> +static void
> +ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions)
> +{
> +    if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
> +        const struct rte_flow_action_mark *mark = actions->conf;
> +
> +        ds_put_cstr(s, "rte flow mark action:\n");
> +        if (mark) {
> +            ds_put_format(s,
> +                          "  Mark: id=%d\n",
> +                          mark->id);
> +        } else {
> +            ds_put_cstr(s, "  Mark = null\n");
> +        }
> +    } else if (actions->type == RTE_FLOW_ACTION_TYPE_RSS) {
> +        const struct rte_flow_action_rss *rss = actions->conf;
> +
> +        ds_put_cstr(s, "rte flow RSS action:\n");
> +        if (rss) {
> +            ds_put_format(s,
> +                          "  RSS: queue_num=%d\n", rss->queue_num);
> +        } else {
> +            ds_put_cstr(s, "  RSS = null\n");
> +        }
> +    } else {
> +        ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
> +    }
> +}
> +
> +struct ds *
> +netdev_dpdk_flow_ds_put_flow(struct ds *s,
> +                             const struct rte_flow_attr *attr,
> +                             const struct rte_flow_item *items,
> +                             const struct rte_flow_action *actions)
> +{
> +    if (attr) {
> +        ds_put_flow_attr(s, attr);
> +    }
> +    while (items && items->type != RTE_FLOW_ITEM_TYPE_END) {
> +        ds_put_flow_pattern(s, items++);
> +    }
> +    while (actions && actions->type != RTE_FLOW_ACTION_TYPE_END) {
> +        ds_put_flow_action(s, actions++);
>      }
>
> -    VLOG_DBG("%s", ds_cstr(&s));
> -    ds_destroy(&s);
> +    return s;
>  }
>
>  static void
> @@ -278,7 +315,6 @@ add_flow_pattern(struct flow_patterns *patterns, enum rte_flow_item_type type,
>      patterns->items[cnt].spec = spec;
>      patterns->items[cnt].mask = mask;
>      patterns->items[cnt].last = NULL;
> -    dump_flow_pattern(&patterns->items[cnt]);
>      patterns->cnt++;
>  }
>
> diff --git a/lib/netdev-offload-dpdk-private.h b/lib/netdev-offload-dpdk-private.h
> index e9c9e602a..68caa7144 100644
> --- a/lib/netdev-offload-dpdk-private.h
> +++ b/lib/netdev-offload-dpdk-private.h
> @@ -51,5 +51,10 @@ void
>  netdev_dpdk_flow_actions_add_mark_rss(struct flow_actions *actions,
>                                        struct netdev *netdev,
>                                        uint32_t mark_id);
> +struct ds *
> +netdev_dpdk_flow_ds_put_flow(struct ds *s,
> +                             const struct rte_flow_attr *attr,
> +                             const struct rte_flow_item *items,
> +                             const struct rte_flow_action *actions);
>
>  #endif /* NETDEV_OFFLOAD_DPDK_PRIVATE_H */
> --
> 2.14.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 02120a379..673cbfbd6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -67,6 +67,7 @@ 
 #include "unixctl.h"
 #include "util.h"
 #include "uuid.h"
+#include "netdev-offload-dpdk-private.h"
 
 enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 
@@ -4452,6 +4453,15 @@  netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
 
     ovs_mutex_lock(&dev->mutex);
     ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+    if (!ret) {
+        VLOG_DBG("Destroy rte_flow %p: netdev=%s, port=%d\n",
+                 rte_flow, netdev_get_name(netdev), dev->port_id);
+    } else {
+        VLOG_ERR("Destroy rte_flow %p: netdev=%s, port=%d\n"
+                 "FAILED. error %u : message : %s",
+                 rte_flow, netdev_get_name(netdev), dev->port_id,
+                 error->type, error->message);
+    }
     ovs_mutex_unlock(&dev->mutex);
     return ret;
 }
@@ -4465,9 +4475,28 @@  netdev_dpdk_rte_flow_create(struct netdev *netdev,
 {
     struct rte_flow *flow;
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+    struct ds s;
 
     ovs_mutex_lock(&dev->mutex);
     flow = rte_flow_create(dev->port_id, attr, items, actions, error);
+    ds_init(&s);
+    if (flow) {
+        VLOG_DBG("Create rte_flow: netdev=%s, port=%d\n"
+                 "%s"
+                 "Flow handle=%p\n",
+                 netdev_get_name(netdev), dev->port_id,
+                 ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
+                                                      actions)), flow);
+    } else {
+        VLOG_ERR("Create rte_flow: netdev=%s, port=%d\n"
+                 "%s"
+                 "FAILED. error %u : message : %s\n",
+                 netdev_get_name(netdev), dev->port_id,
+                 ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
+                                                      actions)),
+                 error->type, error->message);
+    }
+    ds_destroy(&s);
     ovs_mutex_unlock(&dev->mutex);
     return flow;
 }
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index c8c3e28ea..19c933932 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -61,73 +61,71 @@  netdev_dpdk_flow_actions_free(struct flow_actions *actions)
 }
 
 static void
-dump_flow_pattern(struct rte_flow_item *item)
+ds_put_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
 {
-    struct ds s;
-
-    if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
-        return;
-    }
-
-    ds_init(&s);
+    ds_put_format(s,
+                  "  Attributes: "
+                  "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
+                  attr->ingress, attr->egress, attr->priority, attr->group,
+                  attr->transfer);
+}
 
+static void
+ds_put_flow_pattern(struct ds *s, const struct rte_flow_item *item)
+{
     if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
         const struct rte_flow_item_eth *eth_spec = item->spec;
         const struct rte_flow_item_eth *eth_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow eth pattern:\n");
+        ds_put_cstr(s, "rte flow eth pattern:\n");
         if (eth_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
                           "type=0x%04" PRIx16"\n",
                           ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
                           ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
                           ntohs(eth_spec->type));
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (eth_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
                           "type=0x%04"PRIx16"\n",
                           ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
                           ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
                           ntohs(eth_mask->type));
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
         const struct rte_flow_item_vlan *vlan_spec = item->spec;
         const struct rte_flow_item_vlan *vlan_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow vlan pattern:\n");
+        ds_put_cstr(s, "rte flow vlan pattern:\n");
         if (vlan_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
                           ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci));
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
 
         if (vlan_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
                           ntohs(vlan_mask->inner_type), ntohs(vlan_mask->tci));
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
         const struct rte_flow_item_ipv4 *ipv4_spec = item->spec;
         const struct rte_flow_item_ipv4 *ipv4_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow ipv4 pattern:\n");
+        ds_put_cstr(s, "rte flow ipv4 pattern:\n");
         if (ipv4_spec) {
-            ds_put_format(&s,
-                          "  Spec: tos=0x%"PRIx8", ttl=%"PRIx8
+            ds_put_format(s,
+                          "  Spec: tos=0x%"PRIx8", ttl=%"PRIu8
                           ", proto=0x%"PRIx8
                           ", src="IP_FMT", dst="IP_FMT"\n",
                           ipv4_spec->hdr.type_of_service,
@@ -136,11 +134,11 @@  dump_flow_pattern(struct rte_flow_item *item)
                           IP_ARGS(ipv4_spec->hdr.src_addr),
                           IP_ARGS(ipv4_spec->hdr.dst_addr));
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (ipv4_mask) {
-            ds_put_format(&s,
-                          "  Mask: tos=0x%"PRIx8", ttl=%"PRIx8
+            ds_put_format(s,
+                          "  Mask: tos=0x%"PRIx8", ttl=%"PRIu8
                           ", proto=0x%"PRIx8
                           ", src="IP_FMT", dst="IP_FMT"\n",
                           ipv4_mask->hdr.type_of_service,
@@ -149,89 +147,81 @@  dump_flow_pattern(struct rte_flow_item *item)
                           IP_ARGS(ipv4_mask->hdr.src_addr),
                           IP_ARGS(ipv4_mask->hdr.dst_addr));
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
         const struct rte_flow_item_udp *udp_spec = item->spec;
         const struct rte_flow_item_udp *udp_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow udp pattern:\n");
+        ds_put_cstr(s, "rte flow udp pattern:\n");
         if (udp_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16"\n",
                           ntohs(udp_spec->hdr.src_port),
                           ntohs(udp_spec->hdr.dst_port));
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (udp_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: src_port=0x%"PRIx16
                           ", dst_port=0x%"PRIx16"\n",
                           ntohs(udp_mask->hdr.src_port),
                           ntohs(udp_mask->hdr.dst_port));
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_SCTP) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_SCTP) {
         const struct rte_flow_item_sctp *sctp_spec = item->spec;
         const struct rte_flow_item_sctp *sctp_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow sctp pattern:\n");
+        ds_put_cstr(s, "rte flow sctp pattern:\n");
         if (sctp_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16"\n",
                           ntohs(sctp_spec->hdr.src_port),
                           ntohs(sctp_spec->hdr.dst_port));
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (sctp_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: src_port=0x%"PRIx16
                           ", dst_port=0x%"PRIx16"\n",
                           ntohs(sctp_mask->hdr.src_port),
                           ntohs(sctp_mask->hdr.dst_port));
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_ICMP) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_ICMP) {
         const struct rte_flow_item_icmp *icmp_spec = item->spec;
         const struct rte_flow_item_icmp *icmp_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow icmp pattern:\n");
+        ds_put_cstr(s, "rte flow icmp pattern:\n");
         if (icmp_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: icmp_type=%"PRIu8", icmp_code=%"PRIu8"\n",
                           icmp_spec->hdr.icmp_type,
                           icmp_spec->hdr.icmp_code);
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (icmp_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: icmp_type=0x%"PRIx8
                           ", icmp_code=0x%"PRIx8"\n",
                           icmp_spec->hdr.icmp_type,
                           icmp_spec->hdr.icmp_code);
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
-    }
-
-    if (item->type == RTE_FLOW_ITEM_TYPE_TCP) {
+    } else if (item->type == RTE_FLOW_ITEM_TYPE_TCP) {
         const struct rte_flow_item_tcp *tcp_spec = item->spec;
         const struct rte_flow_item_tcp *tcp_mask = item->mask;
 
-        ds_put_cstr(&s, "rte flow tcp pattern:\n");
+        ds_put_cstr(s, "rte flow tcp pattern:\n");
         if (tcp_spec) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Spec: src_port=%"PRIu16", dst_port=%"PRIu16
                           ", data_off=0x%"PRIx8", tcp_flags=0x%"PRIx8"\n",
                           ntohs(tcp_spec->hdr.src_port),
@@ -239,10 +229,10 @@  dump_flow_pattern(struct rte_flow_item *item)
                           tcp_spec->hdr.data_off,
                           tcp_spec->hdr.tcp_flags);
         } else {
-            ds_put_cstr(&s, "  Spec = null\n");
+            ds_put_cstr(s, "  Spec = null\n");
         }
         if (tcp_mask) {
-            ds_put_format(&s,
+            ds_put_format(s,
                           "  Mask: src_port=%"PRIx16", dst_port=%"PRIx16
                           ", data_off=0x%"PRIx8", tcp_flags=0x%"PRIx8"\n",
                           ntohs(tcp_mask->hdr.src_port),
@@ -250,12 +240,59 @@  dump_flow_pattern(struct rte_flow_item *item)
                           tcp_mask->hdr.data_off,
                           tcp_mask->hdr.tcp_flags);
         } else {
-            ds_put_cstr(&s, "  Mask = null\n");
+            ds_put_cstr(s, "  Mask = null\n");
         }
+    } else {
+        ds_put_format(s, "unknown rte flow pattern (%d)\n", item->type);
+    }
+}
+
+static void
+ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions)
+{
+    if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
+        const struct rte_flow_action_mark *mark = actions->conf;
+
+        ds_put_cstr(s, "rte flow mark action:\n");
+        if (mark) {
+            ds_put_format(s,
+                          "  Mark: id=%d\n",
+                          mark->id);
+        } else {
+            ds_put_cstr(s, "  Mark = null\n");
+        }
+    } else if (actions->type == RTE_FLOW_ACTION_TYPE_RSS) {
+        const struct rte_flow_action_rss *rss = actions->conf;
+
+        ds_put_cstr(s, "rte flow RSS action:\n");
+        if (rss) {
+            ds_put_format(s,
+                          "  RSS: queue_num=%d\n", rss->queue_num);
+        } else {
+            ds_put_cstr(s, "  RSS = null\n");
+        }
+    } else {
+        ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
+    }
+}
+
+struct ds *
+netdev_dpdk_flow_ds_put_flow(struct ds *s,
+                             const struct rte_flow_attr *attr,
+                             const struct rte_flow_item *items,
+                             const struct rte_flow_action *actions)
+{
+    if (attr) {
+        ds_put_flow_attr(s, attr);
+    }
+    while (items && items->type != RTE_FLOW_ITEM_TYPE_END) {
+        ds_put_flow_pattern(s, items++);
+    }
+    while (actions && actions->type != RTE_FLOW_ACTION_TYPE_END) {
+        ds_put_flow_action(s, actions++);
     }
 
-    VLOG_DBG("%s", ds_cstr(&s));
-    ds_destroy(&s);
+    return s;
 }
 
 static void
@@ -278,7 +315,6 @@  add_flow_pattern(struct flow_patterns *patterns, enum rte_flow_item_type type,
     patterns->items[cnt].spec = spec;
     patterns->items[cnt].mask = mask;
     patterns->items[cnt].last = NULL;
-    dump_flow_pattern(&patterns->items[cnt]);
     patterns->cnt++;
 }
 
diff --git a/lib/netdev-offload-dpdk-private.h b/lib/netdev-offload-dpdk-private.h
index e9c9e602a..68caa7144 100644
--- a/lib/netdev-offload-dpdk-private.h
+++ b/lib/netdev-offload-dpdk-private.h
@@ -51,5 +51,10 @@  void
 netdev_dpdk_flow_actions_add_mark_rss(struct flow_actions *actions,
                                       struct netdev *netdev,
                                       uint32_t mark_id);
+struct ds *
+netdev_dpdk_flow_ds_put_flow(struct ds *s,
+                             const struct rte_flow_attr *attr,
+                             const struct rte_flow_item *items,
+                             const struct rte_flow_action *actions);
 
 #endif /* NETDEV_OFFLOAD_DPDK_PRIVATE_H */