[ovs-dev] [PATCH v3 ovn 3/4] ovn-northd: Refactor parsing of *_force_snat_ip.
Han Zhou
hzhou at ovn.org
Mon Sep 21 07:01:48 UTC 2020
On Thu, Sep 17, 2020 at 5:51 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Avoid reparsing the *_force_snat_ip addresses for every logical router
port.
> These addresses are defined once for the whole router.
>
The commit message is a little misleading. Originally it wasn't reparsing
for every logical router port. It was per-router. This patch avoids one
extra parsing.
In addition, another minor comment is that the function name
"empty_lport_addresses" may be "lport_addresses_is_empty" to follow the
naming convention of a lot of xxx_is_empty() function names in OVN/OVS.
With these addressed:
Acked-by: Han Zhou <hzhou at ovn.org>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
> lib/ovn-util.c | 6 ++++
> lib/ovn-util.h | 2 +
> northd/ovn-northd.c | 69
++++++++++++++++++++++++---------------------------
> 3 files changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index cdb5e18..a8cf6c9 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -320,6 +320,12 @@ extract_sbrec_binding_first_mac(const struct
sbrec_port_binding *binding,
> return ret;
> }
>
> +bool
> +empty_lport_addresses(struct lport_addresses *laddrs)
> +{
> + return !laddrs->n_ipv4_addrs && !laddrs->n_ipv6_addrs;
> +}
> +
> void
> destroy_lport_addresses(struct lport_addresses *laddrs)
> {
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index d9aadcb..3343f28 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -74,7 +74,7 @@ bool extract_lrp_networks(const struct
nbrec_logical_router_port *,
> struct lport_addresses *);
> bool extract_sbrec_binding_first_mac(const struct sbrec_port_binding
*binding,
> struct eth_addr *ea);
> -
> +bool empty_lport_addresses(struct lport_addresses *);
> void destroy_lport_addresses(struct lport_addresses *);
>
> char *alloc_nat_zone_key(const struct uuid *key, const char *type);
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f79ed99..43bd7b5 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -625,6 +625,8 @@ struct ovn_datapath {
>
> /* SNAT IPs used by the router. */
> struct sset snat_ips;
> + struct lport_addresses dnat_force_snat_addrs;
> + struct lport_addresses lb_force_snat_addrs;
>
> struct ovn_port **localnet_ports;
> size_t n_localnet_ports;
> @@ -670,32 +672,31 @@ nat_entry_is_v6(const struct ovn_nat *nat_entry)
> static void
> init_nat_entries(struct ovn_datapath *od)
> {
> - struct lport_addresses snat_addrs;
> -
> if (!od->nbr) {
> return;
> }
>
> sset_init(&od->snat_ips);
> - if (get_force_snat_ip(od, "dnat", &snat_addrs)) {
> - if (snat_addrs.n_ipv4_addrs) {
> - sset_add(&od->snat_ips, snat_addrs.ipv4_addrs[0].addr_s);
> + if (get_force_snat_ip(od, "dnat", &od->dnat_force_snat_addrs)) {
> + if (&od->dnat_force_snat_addrs.n_ipv4_addrs) {
> + sset_add(&od->snat_ips,
> + od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s);
> }
> - if (snat_addrs.n_ipv6_addrs) {
> - sset_add(&od->snat_ips, snat_addrs.ipv6_addrs[0].addr_s);
> + if (&od->dnat_force_snat_addrs.n_ipv6_addrs) {
> + sset_add(&od->snat_ips,
> + od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s);
> }
> - destroy_lport_addresses(&snat_addrs);
> }
>
> - memset(&snat_addrs, 0, sizeof(snat_addrs));
> - if (get_force_snat_ip(od, "lb", &snat_addrs)) {
> - if (snat_addrs.n_ipv4_addrs) {
> - sset_add(&od->snat_ips, snat_addrs.ipv4_addrs[0].addr_s);
> + if (get_force_snat_ip(od, "lb", &od->lb_force_snat_addrs)) {
> + if (od->lb_force_snat_addrs.n_ipv4_addrs) {
> + sset_add(&od->snat_ips,
> + od->lb_force_snat_addrs.ipv4_addrs[0].addr_s);
> }
> - if (snat_addrs.n_ipv6_addrs) {
> - sset_add(&od->snat_ips, snat_addrs.ipv6_addrs[0].addr_s);
> + if (od->lb_force_snat_addrs.n_ipv6_addrs) {
> + sset_add(&od->snat_ips,
> + od->lb_force_snat_addrs.ipv6_addrs[0].addr_s);
> }
> - destroy_lport_addresses(&snat_addrs);
> }
>
> if (!od->nbr->n_nat) {
> @@ -736,6 +737,9 @@ destroy_nat_entries(struct ovn_datapath *od)
> }
>
> sset_destroy(&od->snat_ips);
> + destroy_lport_addresses(&od->dnat_force_snat_addrs);
> + destroy_lport_addresses(&od->lb_force_snat_addrs);
> +
> for (size_t i = 0; i < od->nbr->n_nat; i++) {
> destroy_lport_addresses(&od->nat_entries[i].ext_addrs);
> }
> @@ -9477,12 +9481,10 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
>
> struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
>
> - struct lport_addresses dnat_force_snat_addrs;
> - struct lport_addresses lb_force_snat_addrs;
> - bool dnat_force_snat_ip = get_force_snat_ip(od, "dnat",
> -
&dnat_force_snat_addrs);
> - bool lb_force_snat_ip = get_force_snat_ip(od, "lb",
> - &lb_force_snat_addrs);
> + bool dnat_force_snat_ip =
> + !empty_lport_addresses(&od->dnat_force_snat_addrs);
> + bool lb_force_snat_ip =
> + !empty_lport_addresses(&od->lb_force_snat_addrs);
>
> for (int i = 0; i < od->nbr->n_nat; i++) {
> const struct nbrec_nat *nat;
> @@ -9982,23 +9984,25 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
> /* Handle force SNAT options set in the gateway router. */
> if (!od->l3dgw_port) {
> if (dnat_force_snat_ip) {
> - if (dnat_force_snat_addrs.n_ipv4_addrs) {
> + if (od->dnat_force_snat_addrs.n_ipv4_addrs) {
> build_lrouter_force_snat_flows(lflows, od, "4",
> - dnat_force_snat_addrs.ipv4_addrs[0].addr_s,
"dnat");
> + od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s,
> + "dnat");
> }
> - if (dnat_force_snat_addrs.n_ipv6_addrs) {
> + if (od->dnat_force_snat_addrs.n_ipv6_addrs) {
> build_lrouter_force_snat_flows(lflows, od, "6",
> - dnat_force_snat_addrs.ipv6_addrs[0].addr_s,
"dnat");
> + od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s,
> + "dnat");
> }
> }
> if (lb_force_snat_ip) {
> - if (lb_force_snat_addrs.n_ipv4_addrs) {
> + if (od->lb_force_snat_addrs.n_ipv4_addrs) {
> build_lrouter_force_snat_flows(lflows, od, "4",
> - lb_force_snat_addrs.ipv4_addrs[0].addr_s, "lb");
> + od->lb_force_snat_addrs.ipv4_addrs[0].addr_s,
"lb");
> }
> - if (lb_force_snat_addrs.n_ipv6_addrs) {
> + if (od->lb_force_snat_addrs.n_ipv6_addrs) {
> build_lrouter_force_snat_flows(lflows, od, "6",
> - lb_force_snat_addrs.ipv6_addrs[0].addr_s, "lb");
> + od->lb_force_snat_addrs.ipv6_addrs[0].addr_s,
"lb");
> }
> }
>
> @@ -10015,13 +10019,6 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
> "ip", "flags.loopback = 1; ct_dnat;");
> }
>
> - if (dnat_force_snat_ip) {
> - destroy_lport_addresses(&dnat_force_snat_addrs);
> - }
> - if (lb_force_snat_ip) {
> - destroy_lport_addresses(&lb_force_snat_addrs);
> - }
> -
> /* Load balancing and packet defrag are only valid on
> * Gateway routers or router with gateway port. */
> if (!smap_get(&od->nbr->options, "chassis") && !od->l3dgw_port) {
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list