[ovs-dev] [PATCH] ofproto/trace: Support for DP-HASH recirculation

Rudra Surya Bhaskara Rao rudrasurya.r at acldigital.com
Tue Jun 8 12:17:52 UTC 2021


Hi Team,

Please consider this as gentle reminder. Could you please review below
changes.

Thanks & Regards,
Surya.

-----Original Message-----
From: Surya Rudra <rudrasurya.r at altencalsoftlabs.com> 
Sent: 23 July 2020 03:15 PM
To: ovs-dev at openvswitch.org
Cc: Surya Rudra <rudrasurya.r at altencalsoftlabs.com>
Subject: [PATCH] ofproto/trace: Support for DP-HASH recirculation

Issue:
Current OVS implementation, recirculation information is not displayed in
ofproto trace command if it happens for dp-hash calculation.

Fix:
Updated ofproto/trace implementation to support dp-hash recirculation.

Test:
Added unit testcase to test the changes.

Signed-off-by: Surya Rudra <rudrasurya.r at altencalsoftlabs.com>
---
 ofproto/ofproto-dpif-trace.c | 19 ++++++++++++++++++-
ofproto/ofproto-dpif-trace.h |  5 ++++-  ofproto/ofproto-dpif-xlate.c | 16
+++++++++++++++-
 tests/ofproto-dpif.at        | 21 +++++++++++++++++++++
 4 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 78a54c7..d558c4b 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -88,7 +88,7 @@ oftrace_add_recirc_node(struct ovs_list *recirc_queue,
                         enum oftrace_recirc_type type, const struct flow
*flow,
                         const struct ofpact_nat *ofn,
                         const struct dp_packet *packet, uint32_t
recirc_id,
-                        const uint16_t zone)
+                        const uint16_t zone, struct ovs_action_hash 
+ *hash_act)
 {
     if (!recirc_id_node_find_and_ref(recirc_id)) {
         return false;
@@ -99,6 +99,7 @@ oftrace_add_recirc_node(struct ovs_list *recirc_queue,
 
     node->type = type;
     node->recirc_id = recirc_id;
+    node->act_hash = hash_act;
     node->flow = *flow;
     node->flow.recirc_id = recirc_id;
     node->flow.ct_zone = zone;
@@ -682,6 +683,22 @@ ofproto_trace_recirc_node(struct oftrace_recirc_node
*node,
             ds_destroy(&s);
         }
         node->flow.ct_state = ct_state;
+    } else if (node->type == OFT_RECIRC_DP_HASH) {
+         const struct ovs_action_hash *hash_act = node->act_hash;
+         uint32_t hash = 0;
+         switch (hash_act->hash_alg) {
+         case OVS_HASH_ALG_SYM_L4:
+             hash = flow_hash_symmetric_l3l4(&node->flow,
+                                             hash_act->hash_basis,
+                                             false);
+             break;
+         case OVS_HASH_ALG_L4:
+         default:
+             hash = flow_hash_5tuple(&node->flow,
+                                     hash_act->hash_basis);
+             break;
+         }
+         node->flow.dp_hash = hash;
     }
     ds_put_char(output, '\n');
 
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index 4b04f17..e50c415 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -54,6 +54,7 @@ enum oftrace_recirc_type {
     OFT_RECIRC_CONNTRACK,
     OFT_RECIRC_MPLS,
     OFT_RECIRC_BOND,
+    OFT_RECIRC_DP_HASH,
 };
 
 /* A node within a trace. */
@@ -74,6 +75,7 @@ struct oftrace_recirc_node {
     struct flow flow;
     struct dp_packet *packet;
     const struct ofpact_nat *nat_act;
+    struct ovs_action_hash *act_hash;
 };
 
 /* A node within a next_ct_states list. */ @@ -94,6 +96,7 @@ bool
oftrace_add_recirc_node(struct ovs_list *recirc_queue,
                              enum oftrace_recirc_type, const struct flow
*,
                              const struct ofpact_nat *,
                              const struct dp_packet *, uint32_t recirc_id,
-                             const uint16_t zone);
+                             const uint16_t zone,
+                             struct ovs_action_hash *hash_act);
 
 #endif /* ofproto-dpif-trace.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e0ede2c..08caacb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4977,6 +4977,20 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t
table)
                                                 sizeof *act_hash);
             act_hash->hash_alg = ctx->dp_hash_alg;
             act_hash->hash_basis = ctx->dp_hash_basis;
+
+            if (OVS_UNLIKELY(ctx->xin->trace) && recirc_id) {
+                if (oftrace_add_recirc_node(ctx->xin->recirc_queue,
+                                  OFT_RECIRC_DP_HASH, &ctx->xin->flow,
NULL,
+                                  ctx->xin->packet, recirc_id, 0,
act_hash)) {
+                    xlate_report(ctx, OFT_DETAIL, "A clone of the packet
is "
+                            "forked to recirculate. The forked pipeline
will "
+                            "be resumed at table %u.", table);
+                } else {
+                    xlate_report(ctx, OFT_DETAIL, "Failed to trace the "
+                            "DP hash forked pipeline with recirc_id =
%d.",
+                            recirc_id);
+                }
+            }
         }
         nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC,
recirc_id);
     }
@@ -5010,7 +5024,7 @@ compose_recirculate_and_fork(struct xlate_ctx *ctx,
uint8_t table,
         if (oftrace_add_recirc_node(ctx->xin->recirc_queue,
                                     OFT_RECIRC_CONNTRACK, &ctx->xin->flow,
                                     ctx->ct_nat_action, ctx->xin->packet,
-                                    recirc_id, zone)) {
+                                    recirc_id, zone, NULL)) {
             xlate_report(ctx, OFT_DETAIL, "A clone of the packet is forked
to "
                          "recirculate. The forked pipeline will be resumed
at "
                          "table %u.", table); diff --git
a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index feabb73..6adb087
100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -627,6 +627,27 @@
recirc_id(0x1),dp_hash(0xXXXX/0xf),in_port(1),packet_type(ns=0,id=0),eth_ty
pe(0x
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - select group in action list]) 
+OVS_VSWITCHD_START add_of_ports br0 1 10 AT_CHECK([ovs-ofctl -O 
+OpenFlow12 add-group br0 
+'group_id=1234,type=select,bucket=set_field:192.168.3.90->ip_src,output
+:10']) AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip 
+actions=group:1234,output:10']) AT_CHECK([ovs-appctl ofproto/trace br0 
+'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x
+0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,
icmp_type=8,icmp_code=0'], [0], [stdout]) AT_CHECK([tail -10 stdout], [0],
+  [    group:1234
+     -> using bucket 0
+    bucket 0
+            set_field:192.168.3.90->ip_src
+            output:10
+    output:10
+
+Final flow: unchanged
+Megaflow: 
+recirc_id=0x1,dp_hash=0xa/0xf,eth,ip,in_port=1,nw_src=192.168.0.1,nw_fr
+ag=no Datapath actions: 
+set(ipv4(src=192.168.3.90)),10,set(ipv4(src=192.168.0.1)),10
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - all group in action set])  OVS_VSWITCHD_START
add_of_ports br0 1 10 11
--
2.7.4


More information about the dev mailing list