[ovs-dev] [PATCH v11 2/5] ovn: avoid snat recirc only on gateway routers
Guru Shetty
guru at ovn.org
Tue Jan 24 22:08:56 UTC 2017
On 21 January 2017 at 16:52, Mickey Spiegel <mickeys.dev at gmail.com> 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 <mickeys.dev at gmail.com>
>
Acked-by: Gurucharan Shetty <guru at ovn.org>
> ---
> include/ovn/actions.h | 3 +++
> ovn/controller/lflow.c | 10 ++++++++++
> ovn/lib/actions.c | 15 +++++++++------
> ovn/ovn-sb.xml | 23 +++++++++++++++++++----
> tests/ovn.at | 2 +-
> 5 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 1d7bd69..d2510fd 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -445,6 +445,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 2d9213b..fa00db2 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,
> @@ -221,6 +230,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 90a2add..fff838b 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -829,12 +829,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/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index f806af7..b33afd3 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1128,11 +1128,26 @@
> <dd>
> <p>
> <code>ct_snat</code> sends the packet through the SNAT zone to
> - unSNAT any packet that was SNATed in the opposite direction.
> If
> - the packet needs to be sent to the next tables, then it
> should be
> - followed by a <code>next;</code> action. The next tables
> will not
> - see the changes in the packet caused by the connection
> tracker.
> + unSNAT any packet that was SNATed in the opposite direction.
> The
> + behavior on gateway routers differs from the behavior on a
> + distributed router:
> </p>
> + <ul>
> + <li>
> + On a gateway router, if the packet needs to be sent to the
> next
> + tables, then it should be followed by a <code>next;</code>
> + action. The next tables will not see the changes in the
> packet
> + caused by the connection tracker.
> + </li>
> + <li>
> + On a distributed router, if the connection tracker finds a
> + connection that was SNATed in the opposite direction, then
> the
> + destination IP address of the packet is UNSNATed. The
> packet is
> + automatically sent to the next tables as if followed by the
> + <code>next;</code> action. The next tables will see the
> changes
> + in the packet caused by the connection tracker.
> + </li>
> + </ul>
> <p>
> <code>ct_snat(<var>IP</var>)</code> sends the packet through
> the
> SNAT zone to change the source IP address of the packet to
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 126574c..6eed020 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -872,7 +872,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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list