[ovs-dev] [PATCH v2] ofproto-dpif-trace: Improve NAT tracing.

Ben Pfaff blp at ovn.org
Tue Jun 16 22:07:54 UTC 2020


On Fri, Jan 10, 2020 at 10:34:43AM +0100, Dumitru Ceara wrote:
> When ofproto/trace detects a recirc action it resumes execution at the
> specified next table. However, if the ct action performs SNAT/DNAT,
> e.g., ct(commit,nat(src=1.1.1.1:4000),table=42), the src/dst IPs and
> ports in the oftrace_recirc_node->flow field are not updated. This leads
> to misleading outputs from ofproto/trace as real packets would actually
> first get NATed and might match different flows when recirculated.
> 
> Assume the first IP/port from the NAT src/dst action will be used by
> conntrack for the translation and update the oftrace_recirc_node->flow
> accordingly. This is not entirely correct as conntrack might choose a
> different IP/port but the result is more realistic than before.
> 
> This fix covers new connections. However, for reply traffic that executes
> actions of the form ct(nat, table=42) we still don't update the flow as
> we don't have any information about conntrack state when tracing.
> 
> Also move the oftrace_recirc_node processing out of ofproto_trace()
> and to its own function, ofproto_trace_recirc_node() for better
> readability/
> 
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>

There are ways to make this better, but it's a good improvement already!

I applied this to master.

I folded in the following minor changes:

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 5f1a05c3a6cc..78a54c715dc7 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -693,15 +693,14 @@ ofproto_trace_recirc_node(struct oftrace_recirc_node *node,
         const struct ofpact_nat *ofn = node->nat_act;
 
         ds_put_cstr(output, "Replacing src/dst IP/ports to simulate NAT:\n");
-        ds_put_cstr(output, " Initial  flow: ");
+        ds_put_cstr(output, " Initial flow: ");
         oftrace_print_ip_flow(&node->flow, ofn->range_af, output);
 
         if (ofn->flags & NX_NAT_F_SRC) {
             if (ofn->range_af == AF_INET) {
                 node->flow.nw_src = ofn->range.addr.ipv4.min;
             } else if (ofn->range_af == AF_INET6) {
-                memcpy(&node->flow.ipv6_src, &ofn->range.addr.ipv6.min,
-                       sizeof node->flow.ipv6_src);
+                node->flow.ipv6_src = ofn->range.addr.ipv6.min;
             }
 
             if (ofn->range_af != AF_UNSPEC && ofn->range.proto.min) {
@@ -712,8 +711,7 @@ ofproto_trace_recirc_node(struct oftrace_recirc_node *node,
             if (ofn->range_af == AF_INET) {
                 node->flow.nw_dst = ofn->range.addr.ipv4.min;
             } else if (ofn->range_af == AF_INET6) {
-                memcpy(&node->flow.ipv6_dst, &ofn->range.addr.ipv6.min,
-                       sizeof node->flow.ipv6_dst);
+                node->flow.ipv6_dst = ofn->range.addr.ipv6.min;
             }
 
             if (ofn->range_af != AF_UNSPEC && ofn->range.proto.min) {

Thanks,

Ben.


More information about the dev mailing list