[ovs-dev] [PATCH] ofproto-dpif-trace: Improve NAT tracing.
dceara at redhat.com
Fri Jan 10 09:46:04 UTC 2020
On Tue, Jan 7, 2020 at 8:25 PM Ben Pfaff <blp at ovn.org> wrote:
> On Tue, Jan 07, 2020 at 09:15:26AM +0100, Dumitru Ceara wrote:
> > On Mon, Jan 6, 2020 at 11:11 PM Ben Pfaff <blp at ovn.org> 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=188.8.131.52: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 <dceara at redhat.com>
> > >
> > > 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.
I sent v2 including your suggestions:
> > 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?
> I have two ideas.
> First, OVS is already able to read and dump the actual connection
> tracking database in use (see e.g. the dpctl/dump-conntrack command in
> ovs-vswitchd(8)). ofproto/trace could consult this. It would want to
> dump out the entry that was found (or not found) while doing it.
This is nice, thanks for pointing it out. So I guess when
troubleshooting packet forwarding we could insert conntrack entries to
> Second, I guess we could have an option whose argument is a set of
> actions to execute, which could update the source and destination, etc.
> That's very general purpose!
I also like this second option because it decouples tracing from the
underlying datapath conntrack implementation.
I'll give it more thought and follow up when I have something working.
More information about the dev