[ovs-dev] [PATCH] ofproto-dpif: Fix NXT_RESUME flow stats

[Yi-Hung Wei]

Currently, OVS does not update the flow stats after a packet is
restarted by NXT_RESUME message.  This patch fixes the aforementioned
issue and adds an unit test to prevent regression.

Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
                      in "continuations".")
VMware-BZ: #2198435
Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
---
 ofproto/ofproto-dpif-xlate.c | 10 ++++++----
 ofproto/ofproto-dpif-xlate.h |  4 ++--
 ofproto/ofproto-dpif.c       | 13 ++++++++++++-
 tests/ofproto-dpif.at        | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 507e14dd0d00..d0e7c6b9f55d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7464,19 +7464,21 @@ enum ofperr
 xlate_resume(struct ofproto_dpif *ofproto,
              const struct ofputil_packet_in_private *pin,
              struct ofpbuf *odp_actions,
-             enum slow_path_reason *slow)
+             enum slow_path_reason *slow,
+             struct flow *flow,
+             struct xlate_cache *xcache)
 {
     struct dp_packet packet;
     dp_packet_use_const(&packet, pin->base.packet,
                         pin->base.packet_len);
 
-    struct flow flow;
-    flow_extract(&packet, &flow);
+    flow_extract(&packet, flow);
 
     struct xlate_in xin;
     xlate_in_init(&xin, ofproto, ofproto_dpif_get_tables_version(ofproto),
-                  &flow, 0, NULL, ntohs(flow.tcp_flags),
+                  flow, 0, NULL, ntohs(flow->tcp_flags),
                   &packet, NULL, odp_actions);
+    xin.xcache = xcache;
 
     struct ofpact_note noop;
     ofpact_init_NOTE(&noop);
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 2cbb3c909a91..0a5a52887b80 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -230,8 +230,8 @@ void xlate_out_uninit(struct xlate_out *);
 
 enum ofperr xlate_resume(struct ofproto_dpif *,
                          const struct ofputil_packet_in_private *,
-                         struct ofpbuf *odp_actions, enum slow_path_reason *);
-
+                         struct ofpbuf *odp_actions, enum slow_path_reason *,
+                         struct flow *, struct xlate_cache *);
 int xlate_send_packet(const struct ofport_dpif *, bool oam, struct dp_packet *);
 
 void xlate_mac_learning_update(const struct ofproto_dpif *ofproto,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0a0c69a7aea0..5ccf23955e6a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5051,18 +5051,29 @@ nxt_resume(struct ofproto *ofproto_,
            const struct ofputil_packet_in_private *pin)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+    struct dpif_flow_stats stats;
+    struct xlate_cache xcache;
+    struct flow flow;
+    xlate_cache_init(&xcache);
 
     /* Translate pin into datapath actions. */
     uint64_t odp_actions_stub[1024 / 8];
     struct ofpbuf odp_actions = OFPBUF_STUB_INITIALIZER(odp_actions_stub);
     enum slow_path_reason slow;
-    enum ofperr error = xlate_resume(ofproto, pin, &odp_actions, &slow);
+    enum ofperr error = xlate_resume(ofproto, pin, &odp_actions, &slow,
+                                     &flow, &xcache);
 
     /* Steal 'pin->packet' and put it into a dp_packet. */
     struct dp_packet packet;
     dp_packet_init(&packet, pin->base.packet_len);
     dp_packet_put(&packet, pin->base.packet, pin->base.packet_len);
 
+    /* Run the side effects from the xcache. */
+    dpif_flow_stats_extract(&flow, &packet, time_msec(), &stats);
+    ovs_mutex_lock(&ofproto_mutex);
+    ofproto_dpif_xcache_execute(ofproto, &xcache, &stats);
+    ovs_mutex_unlock(&ofproto_mutex);
+
     pkt_metadata_from_flow(&packet.md, &pin->base.flow_metadata.flow);
 
     /* Fix up in_port. */
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6b1499f99d78..693a64cab685 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5393,6 +5393,39 @@ CHECK_CONTINUATION([patch ports], [4], [1],
        -- set interface patch11 type=patch options:peer=patch10 \
                                 ofport_request=11])
 
+AT_SETUP([ofproto-dpif - continuation flow stats])
+AT_KEYWORDS([continuations pause resume])
+OVS_VSWITCHD_START
+
+add_of_ports --pcap br0 `seq 1 2`
+
+flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=1 actions=set_field:1->reg1 controller(pause) resubmit(,2)
+table=2 reg1=0x1 actions=2
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows.txt])
+
+AT_CAPTURE_FILE([ofctl_monitor0.log])
+ovs-ofctl monitor br0 resume --detach --no-chdir \
+--pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log
+
+# Run a packet through the switch.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$flow"], [0], [stdout])
+
+# Check flow stats
+AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout])
+AT_CHECK([strip_xids < stdout | sed -n 's/duration=[[0-9]]*\.[[0-9]]*s/duration=0.0s/p' | sed -n 's/idle_age=[[0-9]]*/idle_age=0/p' | grep 'table=2'], [0], [dnl
+ cookie=0x0, duration=0.0s, table=2, n_packets=1, n_bytes=106, idle_age=0, reg1=0x1 actions=output:2
+])
+
+# The packet should be recieved by port 2
+AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap | wc -l`])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
 
 # Check that pause works after the packet is cloned.
 AT_SETUP([ofproto-dpif - continuation after clone])
-- 
2.7.4