On 8 January 2017 at 16:21, Mickey Spiegel <[email protected]> wrote:
> Currently, for performance reasons on gateway routers, ct_snat > that does not specify an IP address does not immediately trigger > recirculation. On gateway routers, ct_snat that does not specify > an IP address happens in the UNSNAT pipeline stage, which is > followed by the DNAT pipeline stage that triggers recirculation > for all packets. This DNAT pipeline stage recirculation takes > care of the recirculation needs of UNSNAT as well as other cases > such as UNDNAT. > > On distributed routers, UNDNAT is handled in the egress pipeline > stage, separately from DNAT in the ingress pipeline stages. The > DNAT pipeline stage only triggers recirculation for some packets. > Due to this difference in design, UNSNAT needs to trigger its own > recirculation. > > This patch restricts the logic that avoids recirculation for > ct_snat, so that it only applies to datapaths representing > gateway routers. > > Signed-off-by: Mickey Spiegel <[email protected]> > The explanation for ct_snat in 'man ovn-sb' will need a change. > --- > include/ovn/actions.h | 3 +++ > ovn/controller/lflow.c | 10 ++++++++++ > ovn/lib/actions.c | 15 +++++++++------ > tests/ovn.at | 2 +- > 4 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 0bf6145..0451c08 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -417,6 +417,9 @@ struct ovnact_encode_params { > /* 'true' if the flow is for a switch. */ > bool is_switch; > > + /* 'true' if the flow is for a gateway router. */ > + bool is_gateway_router; > + > /* A map from a port name to its connection tracking zone. */ > const struct simap *ct_zones; > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > index 3d7633e..2bbbb81 100644 > --- a/ovn/controller/lflow.c > +++ b/ovn/controller/lflow.c > @@ -107,6 +107,15 @@ is_switch(const struct sbrec_datapath_binding *ldp) > > } > > +static bool > +is_gateway_router(const struct sbrec_datapath_binding *ldp, > + const struct hmap *local_datapaths) > +{ > + struct local_datapath *ld = > + get_local_datapath(local_datapaths, ldp->tunnel_key); > + return ld ? ld->has_local_l3gateway : false; > +} > + > /* Adds the logical flows from the Logical_Flow table to flow tables. */ > static void > add_logical_flows(struct controller_ctx *ctx, const struct lport_index > *lports, > @@ -220,6 +229,7 @@ consider_logical_flow(const struct lport_index *lports, > .lookup_port = lookup_port_cb, > .aux = &aux, > .is_switch = is_switch(ldp), > + .is_gateway_router = is_gateway_router(ldp, local_datapaths), > .ct_zones = ct_zones, > .group_table = group_table, > > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c > index 686ecc5..3da3dbe 100644 > --- a/ovn/lib/actions.c > +++ b/ovn/lib/actions.c > @@ -788,12 +788,15 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, > ct = ofpacts->header; > if (cn->ip) { > ct->flags |= NX_CT_F_COMMIT; > - } else if (snat) { > - /* XXX: For performance reasons, we try to prevent additional > - * recirculations. So far, ct_snat which is used in a gateway > router > - * does not need a recirculation. ct_snat(IP) does need a > - * recirculation. Should we consider a method to let the actions > - * specify whether an action needs recirculation if there more use > + } else if (snat && ep->is_gateway_router) { > + /* For performance reasons, we try to prevent additional > + * recirculations. ct_snat which is used in a gateway router > + * does not need a recirculation. ct_snat(IP) does need a > + * recirculation. ct_snat in a distributed router needs > + * recirculation regardless of whether an IP address is > + * specified. > + * XXX Should we consider a method to let the actions specify > + * whether an action needs recirculation if there are more use > * cases?. */ > ct->recirc_table = NX_CT_RECIRC_NONE; > } > diff --git a/tests/ovn.at b/tests/ovn.at > index 5e5d5c2..caa9773 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -852,7 +852,7 @@ ct_dnat(); > > # ct_snat > ct_snat; > - encodes as ct(zone=NXM_NX_REG12[0..15],nat) > + encodes as ct(table=27,zone=NXM_NX_REG12[0..15],nat) > has prereqs ip > ct_snat(192.168.1.2); > encodes as ct(commit,table=27,zone=NXM_NX_REG12[0..15],nat(src=192. > 168.1.2)) > -- > 1.9.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
