diff mbox series

[ovs-dev,V2] Add offload packets statistics

Message ID 20191015065623.30681-1-zhaozhanxu@163.com
State Superseded
Headers show
Series [ovs-dev,V2] Add offload packets statistics | expand

Commit Message

zhaozhanxu Oct. 15, 2019, 6:56 a.m. UTC 
Add argument '-m' for command ovs-appctl bridge/dump-flows
to display the offloaded packets statistics.

The commands display as below:

orignal command:

ovs-appctl bridge/dump-flows br0
duration=243589s, n_packets=214752, n_bytes=223673384, priority=0,actions=NORMAL
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=2,recirc_id=0,actions=drop
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x1,actions=controller(reason=)
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x2,actions=drop
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x3,actions=drop

new command with argument '-m'

ovs-appctl bridge/dump-flows -m br0
duration=243589s, n_packets=140, n_bytes=69284, n_offload_packets=214612,
n_offload_bytes=223604100, priority=0,actions=NORMAL
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=2,recirc_id=0,actions=drop
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x1,actions=controller(reason=)
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x2,actions=drop
table_id=254, duration=243589s, n_packets=0, n_bytes=0, n_offload_packets=0,
n_offload_bytes=0, priority=0,reg0=0x3,actions=drop

Signed-off-by: zhaozhanxu <zhaozhanxu@163.com>
---
 NEWS                               |  2 +
 lib/dpif.h                         |  2 +
 ofproto/bond.c                     |  8 ++--
 ofproto/ofproto-dpif-upcall.c      | 11 +++---
 ofproto/ofproto-dpif-xlate-cache.c |  8 ++--
 ofproto/ofproto-dpif-xlate-cache.h |  6 ++-
 ofproto/ofproto-dpif-xlate.c       |  6 +--
 ofproto/ofproto-dpif.c             | 34 ++++++++++-------
 ofproto/ofproto-dpif.h             |  2 +-
 ofproto/ofproto-provider.h         | 11 +++++-
 ofproto/ofproto.c                  | 59 +++++++++++++++++++-----------
 ofproto/ofproto.h                  |  2 +-
 vswitchd/bridge.c                  | 15 ++++++--
 13 files changed, 105 insertions(+), 61 deletions(-)

Comments

Ben Pfaff Oct. 25, 2019, 12:53 a.m. UTC | #1 
On Tue, Oct 15, 2019 at 02:56:23PM +0800, zhaozhanxu wrote:
> Add argument '-m' for command ovs-appctl bridge/dump-flows
> to display the offloaded packets statistics.

Thanks for the updated patch.

Are n_offload_packets a subset of n_packets, or in addition to them?  If
they are a subset, then n_offload_packets should always be less than or
equal to n_packets; if they are in addition, then there is no
relationship between the two.  Either way, this should be explained in
comments on struct dpif_flow_stats.

I guess that "subset" makes more sense--after all, offloaded packets are
still packets--but then we need to update a few places, like
rule_dpif_credit_stats__().  If we take the "additional" choice, then we
need to change other places: I think most places that n_packets is
referenced, we need to write n_packets + n_offload_packets instead.

Please also update the documentation for bridge/dump-flows in
ovs-vswitchd(8).

get_dpif_flow_stats(), in lib/dpif-netdev.c, should initialize '*stats'.
I don't think it initializes the new members, but it should.

dpif_netdev_flow_put() adds together dpif_flow_stats.  Currently I think
those dpif_flow_stats always have zero in n_offload_*, but it might be a
good idea to add them together anyway.

Actually dpif_netdev_flow_del() does the same thing.  Probably that
means it would be a good idea to factor out the code that adds these
things together into a new helper function since there's already
duplicate code and this adds more of it.

dpif_netlink_flow_get_stats() needs to initialize the new fields.

dpif_flow_stats_extract() needs to initialize the new fields.

I imagine that dpif_flow_stats_format() should display the new
fields--perhaps only if they are nonzero?

Should parse_tc_flower_to_match() set n_offload_* instead of packets and
bytes?  (Or both of them to the same values, if n_offload_* is a subset
of the regular values.)

Ditto for netdev_tc_flow_del().

upcall_xlate() should initialize n_offload_*.

Ditto for revalidate_ukey() and push_dp_ops().

Thanks,

Ben.
zhaozhanxu Oct. 28, 2019, 12:44 p.m. UTC | #2 
Thanks for your reply. I already modified some of them, and I think the others need to discuss.
V2===>V3:
1. Make the n_offload_packets to be a subset of n_packets, and n_offload_bytes to be a subset of n_bytes.
2. Add a new structure 'dpif_flow_detailed_stats' to store the offload statistics, so all the functions you mentioned don't need to modify.


I don't think that function 'parse_tc_flower_to_match' should display new fields, all the commands called function 'parse_tc_flower_to_match' are display the datapath flows.
The datapath flow would be either non-offloaded or offloaded, so that the statistics are offloaded or non-offloaded depends on datapath flows.


The patch link: https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363942.html


At 2019-10-25 08:53:35, "Ben Pfaff" <blp@ovn.org> wrote:
>On Tue, Oct 15, 2019 at 02:56:23PM +0800, zhaozhanxu wrote:
>> Add argument '-m' for command ovs-appctl bridge/dump-flows
>> to display the offloaded packets statistics.
>
>Thanks for the updated patch.
>
>Are n_offload_packets a subset of n_packets, or in addition to them?  If
>they are a subset, then n_offload_packets should always be less than or
>equal to n_packets; if they are in addition, then there is no
>relationship between the two.  Either way, this should be explained in
>comments on struct dpif_flow_stats.
>
>I guess that "subset" makes more sense--after all, offloaded packets are
>still packets--but then we need to update a few places, like
>rule_dpif_credit_stats__().  If we take the "additional" choice, then we
>need to change other places: I think most places that n_packets is
>referenced, we need to write n_packets + n_offload_packets instead.
>
>Please also update the documentation for bridge/dump-flows in
>ovs-vswitchd(8).
>
>get_dpif_flow_stats(), in lib/dpif-netdev.c, should initialize '*stats'.
>I don't think it initializes the new members, but it should.
>
>dpif_netdev_flow_put() adds together dpif_flow_stats.  Currently I think
>those dpif_flow_stats always have zero in n_offload_*, but it might be a
>good idea to add them together anyway.
>
>Actually dpif_netdev_flow_del() does the same thing.  Probably that
>means it would be a good idea to factor out the code that adds these
>things together into a new helper function since there's already
>duplicate code and this adds more of it.
>
>dpif_netlink_flow_get_stats() needs to initialize the new fields.
>
>dpif_flow_stats_extract() needs to initialize the new fields.
>
>I imagine that dpif_flow_stats_format() should display the new
>fields--perhaps only if they are nonzero?
>
>Should parse_tc_flower_to_match() set n_offload_* instead of packets and
>bytes?  (Or both of them to the same values, if n_offload_* is a subset
>of the regular values.)
>
>Ditto for netdev_tc_flow_del().
>
>upcall_xlate() should initialize n_offload_*.
>
>Ditto for revalidate_ukey() and push_dp_ops().
>
>Thanks,
>
>Ben.
zhaozhanxu Nov. 26, 2019, 7:56 a.m. UTC | #3 
Please help me reviewing the code, thanks very much.






At 2019-10-28 20:44:51, "zhaozhanxu" <zhaozhanxu@163.com> wrote:

Thanks for your reply. I already modified some of them, and I think the others need to discuss.
V2===>V3:
1. Make the n_offload_packets to be a subset of n_packets, and n_offload_bytes to be a subset of n_bytes.
2. Add a new structure 'dpif_flow_detailed_stats' to store the offload statistics, so all the functions you mentioned don't need to modify.


I don't think that function 'parse_tc_flower_to_match' should display new fields, all the commands called function 'parse_tc_flower_to_match' are display the datapath flows.
The datapath flow would be either non-offloaded or offloaded, so that the statistics are offloaded or non-offloaded depends on datapath flows.


The patch link: https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363942.html


At 2019-10-25 08:53:35, "Ben Pfaff" <blp@ovn.org> wrote:
>On Tue, Oct 15, 2019 at 02:56:23PM +0800, zhaozhanxu wrote:
>> Add argument '-m' for command ovs-appctl bridge/dump-flows
>> to display the offloaded packets statistics.
>
>Thanks for the updated patch.
>
>Are n_offload_packets a subset of n_packets, or in addition to them?  If
>they are a subset, then n_offload_packets should always be less than or
>equal to n_packets; if they are in addition, then there is no
>relationship between the two.  Either way, this should be explained in
>comments on struct dpif_flow_stats.
>
>I guess that "subset" makes more sense--after all, offloaded packets are
>still packets--but then we need to update a few places, like
>rule_dpif_credit_stats__().  If we take the "additional" choice, then we
>need to change other places: I think most places that n_packets is
>referenced, we need to write n_packets + n_offload_packets instead.
>
>Please also update the documentation for bridge/dump-flows in
>ovs-vswitchd(8).
>
>get_dpif_flow_stats(), in lib/dpif-netdev.c, should initialize '*stats'.
>I don't think it initializes the new members, but it should.
>
>dpif_netdev_flow_put() adds together dpif_flow_stats.  Currently I think
>those dpif_flow_stats always have zero in n_offload_*, but it might be a
>good idea to add them together anyway.
>
>Actually dpif_netdev_flow_del() does the same thing.  Probably that
>means it would be a good idea to factor out the code that adds these
>things together into a new helper function since there's already
>duplicate code and this adds more of it.
>
>dpif_netlink_flow_get_stats() needs to initialize the new fields.
>
>dpif_flow_stats_extract() needs to initialize the new fields.
>
>I imagine that dpif_flow_stats_format() should display the new
>fields--perhaps only if they are nonzero?
>
>Should parse_tc_flower_to_match() set n_offload_* instead of packets and
>bytes?  (Or both of them to the same values, if n_offload_* is a subset
>of the regular values.)
>
>Ditto for netdev_tc_flow_del().
>
>upcall_xlate() should initialize n_offload_*.
>
>Ditto for revalidate_ukey() and push_dp_ops().
>
>Thanks,
>
>Ben.
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 330ab3832..bf36c3520 100644
--- a/NEWS
+++ b/NEWS
@@ -81,6 +81,8 @@  v2.12.0 - 03 Sep 2019
      * Add support for conntrack zone-based timeout policy.
    - 'ovs-dpctl dump-flows' is no longer suitable for dumping offloaded flows.
      'ovs-appctl dpctl/dump-flows' should be used instead.
+   - Add new argument '-m' for command 'ovs-appctl bridge/dump-flows',
+     so it can display offloaded packets statistics.
    - Add L2 GRE tunnel over IPv6 support.
 
 v2.11.0 - 19 Feb 2019
diff --git a/lib/dpif.h b/lib/dpif.h
index 289d574a0..9a63398a2 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -492,6 +492,8 @@  void dpif_port_poll_wait(const struct dpif *);
 struct dpif_flow_stats {
     uint64_t n_packets;
     uint64_t n_bytes;
+    uint64_t n_offload_packets;
+    uint64_t n_offload_bytes;
     long long int used;
     uint16_t tcp_flags;
 };
diff --git a/ofproto/bond.c b/ofproto/bond.c
index c5d5f2c03..66642792c 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -946,13 +946,13 @@  bond_recirculation_account(struct bond *bond)
         struct rule *rule = entry->pr_rule;
 
         if (rule) {
-            uint64_t n_packets OVS_UNUSED;
+            struct pkts_stats stats;
             long long int used OVS_UNUSED;
-            uint64_t n_bytes;
 
             rule->ofproto->ofproto_class->rule_get_stats(
-                rule, &n_packets, &n_bytes, &used);
-            bond_entry_account(entry, n_bytes);
+                rule, &stats, &used);
+            bond_entry_account(entry,
+                               stats.n_bytes + stats.n_offload_bytes);
         }
     }
 }
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 657aa7f79..5fdba5b65 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2260,7 +2260,7 @@  static enum reval_result
 revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
                 const struct dpif_flow_stats *stats,
                 struct ofpbuf *odp_actions, uint64_t reval_seq,
-                struct recirc_refs *recircs)
+                struct recirc_refs *recircs, bool offloaded)
     OVS_REQUIRES(ukey->mutex)
 {
     bool need_revalidate = ukey->reval_seq != reval_seq;
@@ -2295,7 +2295,7 @@  revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
 
     /* Stats for deleted flows will be attributed upon flow deletion. Skip. */
     if (result != UKEY_DELETE) {
-        xlate_push_stats(ukey->xcache, &push);
+        xlate_push_stats(ukey->xcache, &push, offloaded);
         ukey->stats = *stats;
         ukey->reval_seq = reval_seq;
     }
@@ -2408,7 +2408,7 @@  push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)
             if (op->ukey) {
                 ovs_mutex_lock(&op->ukey->mutex);
                 if (op->ukey->xcache) {
-                    xlate_push_stats(op->ukey->xcache, push);
+                    xlate_push_stats(op->ukey->xcache, push, false);
                     ovs_mutex_unlock(&op->ukey->mutex);
                     continue;
                 }
@@ -2685,7 +2685,8 @@  revalidate(struct revalidator *revalidator)
                 result = UKEY_DELETE;
             } else {
                 result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
-                                         reval_seq, &recircs);
+                                         reval_seq, &recircs,
+                                         f->attrs.offloaded);
             }
             ukey->dump_seq = dump_seq;
 
@@ -2770,7 +2771,7 @@  revalidator_sweep__(struct revalidator *revalidator, bool purge)
                     COVERAGE_INC(revalidate_missed_dp_flow);
                     memset(&stats, 0, sizeof stats);
                     result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
-                                             reval_seq, &recircs);
+                                             reval_seq, &recircs, false);
                 }
                 if (result != UKEY_KEEP) {
                     /* Clears 'recircs' if filled by revalidate_ukey(). */
diff --git a/ofproto/ofproto-dpif-xlate-cache.c b/ofproto/ofproto-dpif-xlate-cache.c
index 2a2e77a42..dcc91cb38 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -90,7 +90,7 @@  xlate_cache_netdev(struct xc_entry *entry, const struct dpif_flow_stats *stats)
 /* Push stats and perform side effects of flow translation. */
 void
 xlate_push_stats_entry(struct xc_entry *entry,
-                       struct dpif_flow_stats *stats)
+                       struct dpif_flow_stats *stats, bool offloaded)
 {
     struct eth_addr dmac;
 
@@ -104,7 +104,7 @@  xlate_push_stats_entry(struct xc_entry *entry,
                                         ? 0 : stats->n_packets);
         break;
     case XC_RULE:
-        rule_dpif_credit_stats(entry->rule, stats);
+        rule_dpif_credit_stats(entry->rule, stats, offloaded);
         break;
     case XC_BOND:
         bond_account(entry->bond.bond, entry->bond.flow,
@@ -169,7 +169,7 @@  xlate_push_stats_entry(struct xc_entry *entry,
 
 void
 xlate_push_stats(struct xlate_cache *xcache,
-                 struct dpif_flow_stats *stats)
+                 struct dpif_flow_stats *stats, bool offloaded)
 {
     if (!stats->n_packets) {
         return;
@@ -178,7 +178,7 @@  xlate_push_stats(struct xlate_cache *xcache,
     struct xc_entry *entry;
     struct ofpbuf entries = xcache->entries;
     XC_ENTRY_FOR_EACH (entry, &entries) {
-        xlate_push_stats_entry(entry, stats);
+        xlate_push_stats_entry(entry, stats, offloaded);
     }
 }
 
diff --git a/ofproto/ofproto-dpif-xlate-cache.h b/ofproto/ofproto-dpif-xlate-cache.h
index e5dae6dc6..114aff8ea 100644
--- a/ofproto/ofproto-dpif-xlate-cache.h
+++ b/ofproto/ofproto-dpif-xlate-cache.h
@@ -142,8 +142,10 @@  struct xlate_cache {
 void xlate_cache_init(struct xlate_cache *);
 struct xlate_cache *xlate_cache_new(void);
 struct xc_entry *xlate_cache_add_entry(struct xlate_cache *, enum xc_type);
-void xlate_push_stats_entry(struct xc_entry *, struct dpif_flow_stats *);
-void xlate_push_stats(struct xlate_cache *, struct dpif_flow_stats *);
+void xlate_push_stats_entry(struct xc_entry *, struct dpif_flow_stats *,
+                            bool);
+void xlate_push_stats(struct xlate_cache *, struct dpif_flow_stats *,
+                      bool);
 void xlate_cache_clear_entry(struct xc_entry *);
 void xlate_cache_clear(struct xlate_cache *);
 void xlate_cache_uninit(struct xlate_cache *);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f92cb62c8..8db7b0e41 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3697,7 +3697,7 @@  native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
          */
         if (backup_resubmit_stats) {
             struct dpif_flow_stats stats = *backup_resubmit_stats;
-            xlate_push_stats(ctx->xin->xcache, &stats);
+            xlate_push_stats(ctx->xin->xcache, &stats, false);
         }
         xlate_cache_steal_entries(backup_xcache, ctx->xin->xcache);
 
@@ -4263,7 +4263,7 @@  xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule,
     const struct rule_actions *actions;
 
     if (ctx->xin->resubmit_stats) {
-        rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats);
+        rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats, false);
     }
 
     ctx->resubmits++;
@@ -7589,7 +7589,7 @@  xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
             ctx.xin->resubmit_stats, &ctx.table_id,
             flow->in_port.ofp_port, true, true, ctx.xin->xcache);
         if (ctx.xin->resubmit_stats) {
-            rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats);
+            rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats, false);
         }
         if (ctx.xin->xcache) {
             struct xc_entry *entry;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 496a16c8a..f59ba4385 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -80,7 +80,7 @@  COVERAGE_DEFINE(packet_in_overflow);
 
 struct flow_miss;
 
-static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t *bytes,
+static void rule_get_stats(struct rule *, struct pkts_stats *stats,
                            long long int *used);
 static struct rule_dpif *rule_dpif_cast(const struct rule *);
 static void rule_expire(struct rule_dpif *, long long now);
@@ -4115,7 +4115,7 @@  ofproto_dpif_execute_actions__(struct ofproto_dpif *ofproto,
     dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
 
     if (rule) {
-        rule_dpif_credit_stats(rule, &stats);
+        rule_dpif_credit_stats(rule, &stats, false);
     }
 
     uint64_t odp_actions_stub[1024 / 8];
@@ -4169,27 +4169,33 @@  ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto,
 static void
 rule_dpif_credit_stats__(struct rule_dpif *rule,
                          const struct dpif_flow_stats *stats,
-                         bool credit_counts)
+                         bool credit_counts, bool offloaded)
     OVS_REQUIRES(rule->stats_mutex)
 {
     if (credit_counts) {
-        rule->stats.n_packets += stats->n_packets;
-        rule->stats.n_bytes += stats->n_bytes;
+        if (offloaded) {
+            rule->stats.n_offload_packets += stats->n_packets;
+            rule->stats.n_offload_bytes += stats->n_bytes;
+        } else {
+            rule->stats.n_packets += stats->n_packets;
+            rule->stats.n_bytes += stats->n_bytes;
+        }
     }
     rule->stats.used = MAX(rule->stats.used, stats->used);
 }
 
 void
 rule_dpif_credit_stats(struct rule_dpif *rule,
-                       const struct dpif_flow_stats *stats)
+                       const struct dpif_flow_stats *stats, bool offloaded)
 {
     ovs_mutex_lock(&rule->stats_mutex);
     if (OVS_UNLIKELY(rule->new_rule)) {
         ovs_mutex_lock(&rule->new_rule->stats_mutex);
-        rule_dpif_credit_stats__(rule->new_rule, stats, rule->forward_counts);
+        rule_dpif_credit_stats__(rule->new_rule, stats, rule->forward_counts,
+                                 offloaded);
         ovs_mutex_unlock(&rule->new_rule->stats_mutex);
     } else {
-        rule_dpif_credit_stats__(rule, stats, true);
+        rule_dpif_credit_stats__(rule, stats, true, offloaded);
     }
     ovs_mutex_unlock(&rule->stats_mutex);
 }
@@ -4640,17 +4646,19 @@  rule_destruct(struct rule *rule_)
 }
 
 static void
-rule_get_stats(struct rule *rule_, uint64_t *packets, uint64_t *bytes,
+rule_get_stats(struct rule *rule_, struct pkts_stats *stats,
                long long int *used)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
 
     ovs_mutex_lock(&rule->stats_mutex);
     if (OVS_UNLIKELY(rule->new_rule)) {
-        rule_get_stats(&rule->new_rule->up, packets, bytes, used);
+        rule_get_stats(&rule->new_rule->up, stats, used);
     } else {
-        *packets = rule->stats.n_packets;
-        *bytes = rule->stats.n_bytes;
+        stats->n_packets = rule->stats.n_packets;
+        stats->n_bytes = rule->stats.n_bytes;
+        stats->n_offload_packets = rule->stats.n_offload_packets;
+        stats->n_offload_bytes = rule->stats.n_offload_bytes;
         *used = rule->stats.used;
     }
     ovs_mutex_unlock(&rule->stats_mutex);
@@ -4814,7 +4822,7 @@  ofproto_dpif_xcache_execute(struct ofproto_dpif *ofproto,
         case XC_GROUP:
         case XC_TNL_NEIGH:
         case XC_TUNNEL_HEADER:
-            xlate_push_stats_entry(entry, stats);
+            xlate_push_stats_entry(entry, stats, false);
             break;
         default:
             OVS_NOT_REACHED();
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index cd453636c..c9daa2eb9 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -107,7 +107,7 @@  struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *,
                                               struct xlate_cache *);
 
 void rule_dpif_credit_stats(struct rule_dpif *,
-                            const struct dpif_flow_stats *);
+                            const struct dpif_flow_stats *, bool);
 
 void rule_set_recirc_id(struct rule *, uint32_t id);
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c980e6bff..819ea89e3 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -588,6 +588,13 @@  struct ofgroup {
     struct rule_collection rules OVS_GUARDED;   /* Referring rules. */
 };
 
+struct pkts_stats {
+    uint64_t n_packets;
+    uint64_t n_bytes;
+    uint64_t n_offload_packets;
+    uint64_t n_offload_bytes;
+};
+
 struct ofgroup *ofproto_group_lookup(const struct ofproto *ofproto,
                                      uint32_t group_id, ovs_version_t version,
                                      bool take_ref);
@@ -1348,8 +1355,8 @@  struct ofproto_class {
      * matched it in '*packet_count' and the number of bytes in those packets
      * in '*byte_count'.  UINT64_MAX indicates that the packet count or byte
      * count is unknown. */
-    void (*rule_get_stats)(struct rule *rule, uint64_t *packet_count,
-                           uint64_t *byte_count, long long int *used)
+    void (*rule_get_stats)(struct rule *rule, struct pkts_stats *stats,
+                           long long int *used)
         /* OVS_EXCLUDED(ofproto_mutex) */;
 
     /* Translates actions in 'opo->ofpacts', for 'opo->packet' in flow tables
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b4249b0d8..674e296c6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4640,6 +4640,7 @@  handle_flow_stats_request(struct ofconn *ofconn,
     RULE_COLLECTION_FOR_EACH (rule, &rules) {
         long long int now = time_msec();
         struct ofputil_flow_stats fs;
+        struct pkts_stats stats;
         long long int created, used, modified;
         const struct rule_actions *actions;
         enum ofputil_flow_mod_flags flags;
@@ -4655,8 +4656,9 @@  handle_flow_stats_request(struct ofconn *ofconn,
         flags = rule->flags;
         ovs_mutex_unlock(&rule->mutex);
 
-        ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
-                                               &fs.byte_count, &used);
+        ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
+        fs.packet_count = stats.n_packets + stats.n_offload_packets;
+        fs.byte_count = stats.n_bytes + stats.n_offload_bytes;
 
         minimatch_expand(&rule->cr.match, &fs.match);
         fs.table_id = rule->table_id;
@@ -4681,14 +4683,14 @@  handle_flow_stats_request(struct ofconn *ofconn,
 }
 
 static void
-flow_stats_ds(struct ofproto *ofproto, struct rule *rule, struct ds *results)
+flow_stats_ds(struct ofproto *ofproto, struct rule *rule, struct ds *results,
+              bool verbosity)
 {
-    uint64_t packet_count, byte_count;
+    struct pkts_stats stats;
     const struct rule_actions *actions;
     long long int created, used;
 
-    rule->ofproto->ofproto_class->rule_get_stats(rule, &packet_count,
-                                                 &byte_count, &used);
+    rule->ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
 
     ovs_mutex_lock(&rule->mutex);
     actions = rule_get_actions(rule);
@@ -4699,8 +4701,19 @@  flow_stats_ds(struct ofproto *ofproto, struct rule *rule, struct ds *results)
         ds_put_format(results, "table_id=%"PRIu8", ", rule->table_id);
     }
     ds_put_format(results, "duration=%llds, ", (time_msec() - created) / 1000);
-    ds_put_format(results, "n_packets=%"PRIu64", ", packet_count);
-    ds_put_format(results, "n_bytes=%"PRIu64", ", byte_count);
+    if (verbosity) {
+        ds_put_format(results, "n_packets=%"PRIu64", ", stats.n_packets);
+        ds_put_format(results, "n_bytes=%"PRIu64", ", stats.n_bytes);
+        ds_put_format(results, "n_offload_packets=%"PRIu64", ",
+                      stats.n_offload_packets);
+        ds_put_format(results, "n_offload_bytes=%"PRIu64", ",
+                      stats.n_offload_bytes);
+    } else {
+        ds_put_format(results, "n_packets=%"PRIu64", ",
+                      stats.n_packets + stats.n_offload_packets);
+        ds_put_format(results, "n_bytes=%"PRIu64", ",
+                      stats.n_bytes + stats.n_offload_packets);
+    }
     cls_rule_format(&rule->cr, ofproto_get_tun_tab(ofproto), NULL, results);
     ds_put_char(results, ',');
 
@@ -4714,7 +4727,7 @@  flow_stats_ds(struct ofproto *ofproto, struct rule *rule, struct ds *results)
 /* Adds a pretty-printed description of all flows to 'results', including
  * hidden flows (e.g., set up by in-band control). */
 void
-ofproto_get_all_flows(struct ofproto *p, struct ds *results)
+ofproto_get_all_flows(struct ofproto *p, struct ds *results, bool verbosity)
 {
     struct oftable *table;
 
@@ -4722,7 +4735,7 @@  ofproto_get_all_flows(struct ofproto *p, struct ds *results)
         struct rule *rule;
 
         CLS_FOR_EACH (rule, cr, &table->cls) {
-            flow_stats_ds(p, rule, results);
+            flow_stats_ds(p, rule, results, verbosity);
         }
     }
 }
@@ -4810,23 +4823,23 @@  handle_aggregate_stats_request(struct ofconn *ofconn,
 
     struct rule *rule;
     RULE_COLLECTION_FOR_EACH (rule, &rules) {
-        uint64_t packet_count;
-        uint64_t byte_count;
+        struct pkts_stats pkts_stats;
         long long int used;
 
-        ofproto->ofproto_class->rule_get_stats(rule, &packet_count,
-                                               &byte_count, &used);
+        ofproto->ofproto_class->rule_get_stats(rule, &pkts_stats, &used);
 
-        if (packet_count == UINT64_MAX) {
+        if (pkts_stats.n_packets == UINT64_MAX) {
             unknown_packets = true;
         } else {
-            stats.packet_count += packet_count;
+            stats.packet_count += pkts_stats.n_packets;
+            stats.packet_count += pkts_stats.n_offload_packets;
         }
 
-        if (byte_count == UINT64_MAX) {
+        if (pkts_stats.n_bytes == UINT64_MAX) {
             unknown_bytes = true;
         } else {
-            stats.byte_count += byte_count;
+            stats.byte_count += pkts_stats.n_bytes;
+            stats.byte_count += pkts_stats.n_offload_bytes;
         }
 
         stats.flow_count++;
@@ -6017,6 +6030,7 @@  ofproto_rule_send_removed(struct rule *rule)
 {
     struct ofputil_flow_removed fr;
     long long int used;
+    struct pkts_stats stats;
 
     minimatch_expand(&rule->cr.match, &fr.match);
     fr.priority = rule->cr.priority;
@@ -6039,8 +6053,9 @@  ofproto_rule_send_removed(struct rule *rule)
     fr.idle_timeout = rule->idle_timeout;
     fr.hard_timeout = rule->hard_timeout;
     ovs_mutex_unlock(&rule->mutex);
-    rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count,
-                                                 &fr.byte_count, &used);
+    rule->ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
+    fr.packet_count += stats.n_packets + stats.n_offload_packets;
+    fr.byte_count += stats.n_bytes + stats.n_offload_bytes;
     connmgr_send_flow_removed(connmgr, &fr);
     ovs_mutex_unlock(&ofproto_mutex);
 }
@@ -8850,11 +8865,11 @@  rule_eviction_priority(struct ofproto *ofproto, struct rule *rule)
         expiration = modified + rule->hard_timeout * 1000;
     }
     if (rule->idle_timeout) {
-        uint64_t packets, bytes;
+        struct pkts_stats stats;
         long long int used;
         long long int idle_expiration;
 
-        ofproto->ofproto_class->rule_get_stats(rule, &packets, &bytes, &used);
+        ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
         idle_expiration = used + rule->idle_timeout * 1000;
         expiration = MIN(expiration, idle_expiration);
     }
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 033c4cf93..8924baf29 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -541,7 +541,7 @@  void ofproto_configure_table(struct ofproto *, int table_id,
 /* Configuration querying. */
 bool ofproto_has_snoops(const struct ofproto *);
 void ofproto_get_snoops(const struct ofproto *, struct sset *);
-void ofproto_get_all_flows(struct ofproto *p, struct ds *);
+void ofproto_get_all_flows(struct ofproto *p, struct ds *, bool);
 void ofproto_get_netflow_ids(const struct ofproto *,
                              uint8_t *engine_type, uint8_t *engine_id);
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9095ebf5d..a4f7486a1 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -514,7 +514,7 @@  bridge_init(const char *remote)
                              qos_unixctl_show_types, NULL);
     unixctl_command_register("qos/show", "interface", 1, 1,
                              qos_unixctl_show, NULL);
-    unixctl_command_register("bridge/dump-flows", "bridge", 1, 1,
+    unixctl_command_register("bridge/dump-flows", "[-m] bridge", 1, 2,
                              bridge_unixctl_dump_flows, NULL);
     unixctl_command_register("bridge/reconnect", "[bridge]", 0, 1,
                              bridge_unixctl_reconnect, NULL);
@@ -3573,20 +3573,27 @@  bridge_lookup(const char *name)
 /* Handle requests for a listing of all flows known by the OpenFlow
  * stack, including those normally hidden. */
 static void
-bridge_unixctl_dump_flows(struct unixctl_conn *conn, int argc OVS_UNUSED,
+bridge_unixctl_dump_flows(struct unixctl_conn *conn, int argc,
                           const char *argv[], void *aux OVS_UNUSED)
 {
     struct bridge *br;
     struct ds results;
 
-    br = bridge_lookup(argv[1]);
+    br = bridge_lookup(argv[argc - 1]);
     if (!br) {
         unixctl_command_reply_error(conn, "Unknown bridge");
         return;
     }
 
+    bool verbosity = false;
+    for (int i = 1; i < argc - 1; i++) {
+        if (!strcmp(argv[i], "-m")) {
+            verbosity = true;
+        }
+    }
+
     ds_init(&results);
-    ofproto_get_all_flows(br->ofproto, &results);
+    ofproto_get_all_flows(br->ofproto, &results, verbosity);
 
     unixctl_command_reply(conn, ds_cstr(&results));
     ds_destroy(&results);