On Mon, Jan 6, 2020 at 11:11 PM Ben Pfaff <[email protected]> wrote: > > On Fri, Jan 03, 2020 at 05:27:31PM +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. > > > > Signed-off-by: Dumitru Ceara <[email protected]> > > This is a great idea. > > I have an idea for further improvement. Currently this patch is quiet: > it doesn't say what's going on. It would be better if it said that it > was replacing the source and/or destination addresses, and by what and > why. I think that would be easiest done in ofproto_trace() itself near > the existing code that does it already for ct_state. I think we could > just save the ofn pointer in the recirc node and move the replacement > code to ofproto_trace(). >
Thanks for the review! Sounds better indeed. I'll respin the patch with your enhancement. As follow up work I was thinking it would be nice to be able to cover the other types of nat recirculation too, e.g., ct(nat, table=42), and allow the user to specify the outcome of the nat operation in a similar way as we do for conntrack with --ct-next. However, I'm not sure what the best way is to do that.. I'm open to suggestions. Could we enhance ofproto/trace such that the user can specify a conntrack database in a predefined format? For example, the output of "conntrack -L -o xml". Or would that become confusing? Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
