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 <[email protected]>
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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
