| Message ID | 20241008101155.331151-2-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Commit all traffic for stateful NAT/LB | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Tue, Oct 8, 2024 at 6:12 AM Ales Musil <amusil@redhat.com> wrote: > > Most of the functions that are building logical flows > for NATs did type comparison via strcmp. Store the type > in enum instead in the nat record and compare the enum instead. > > Signed-off-by: Ales Musil <amusil@redhat.com> Thanks for the patch. It makes sense to do this way, Applied the patch to main. Numan > --- > northd/en-lr-nat.c | 10 +++-- > northd/en-lr-nat.h | 7 +++ > northd/northd.c | 110 ++++++++++++++++++++++++--------------------- > 3 files changed, 74 insertions(+), 53 deletions(-) > > diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c > index 215d924e4..bdbb2c860 100644 > --- a/northd/en-lr-nat.c > +++ b/northd/en-lr-nat.c > @@ -313,10 +313,14 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > nat_entry->ext_addrs.ipv6_addrs[0].addr_s, > nat_entry); > } > + nat_entry->type = SNAT; > } else { > - if (!strcmp(nat->type, "dnat_and_snat") > - && nat->logical_port && nat->external_mac) { > - lrnat_rec->has_distributed_nat = true; > + nat_entry->type = DNAT; > + if (!strcmp(nat->type, "dnat_and_snat")) { > + nat_entry->type = DNAT_AND_SNAT; > + if (nat->logical_port && nat->external_mac) { > + lrnat_rec->has_distributed_nat = true; > + } > } > > if (nat->external_mac) { > diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h > index 81a7b0abd..120ceeca9 100644 > --- a/northd/en-lr-nat.h > +++ b/northd/en-lr-nat.h > @@ -29,6 +29,12 @@ > #include "lib/ovn-sb-idl.h" > #include "lib/ovn-util.h" > > +enum ovn_nat_type { > + SNAT, > + DNAT, > + DNAT_AND_SNAT, > +}; > + > /* Contains a NAT entry with the external addresses pre-parsed. */ > struct ovn_nat { > const struct nbrec_nat *nb; > @@ -39,6 +45,7 @@ struct ovn_nat { > */ > bool is_router_ip; /* Indicates if the NAT external_ip is also one of > * router's lrp ip. Can be 'true' only for SNAT. */ > + enum ovn_nat_type type; > }; > > /* Stores the list of SNAT entries referencing a unique SNAT IP address. > diff --git a/northd/northd.c b/northd/northd.c > index 2c4703301..0364dd766 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -8950,7 +8950,7 @@ build_lswitch_rport_arp_req_flows_for_lbnats( > continue; > } > > - if (!strcmp(nat->type, "snat")) { > + if (nat_entry->type == SNAT) { > continue; > } > > @@ -10356,8 +10356,8 @@ build_lswitch_ip_unicast_lookup_for_nats( > const struct ovn_nat *nat = > &lr_stateful_rec->lrnat_rec->nat_entries[i]; > > - if (!strcmp(nat->nb->type, "dnat_and_snat") > - && nat->nb->logical_port && nat->nb->external_mac > + if (nat->type == DNAT_AND_SNAT && nat->nb->logical_port > + && nat->nb->external_mac > && eth_addr_from_string(nat->nb->external_mac, &mac)) { > > ds_clear(match); > @@ -12307,10 +12307,10 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) > } > > static inline bool > -lrouter_dnat_and_snat_is_stateless(const struct nbrec_nat *nat) > +lrouter_dnat_and_snat_is_stateless(const struct ovn_nat *nat) > { > - return smap_get_bool(&nat->options, "stateless", false) && > - !strcmp(nat->type, "dnat_and_snat"); > + return smap_get_bool(&nat->nb->options, "stateless", false) && > + nat->type == DNAT_AND_SNAT; > } > > #define NAT_PRIORITY_MATCH_OFFSET 300 > @@ -14587,7 +14587,7 @@ build_lr_gateway_redirect_flows_for_nats( > for (int j = 0; j < lrnat_rec->n_nat_entries; j++) { > const struct ovn_nat *nat = &lrnat_rec->nat_entries[j]; > > - if (!lrouter_dnat_and_snat_is_stateless(nat->nb) || > + if (!lrouter_dnat_and_snat_is_stateless(nat) || > (!nat->nb->allowed_ext_ips && !nat->nb->exempted_ext_ips)) { > continue; > } > @@ -15237,7 +15237,7 @@ build_lrouter_arp_nd_for_datapath(const struct ovn_datapath *od, > /* Skip SNAT entries for now, we handle unique SNAT IPs separately > * below. > */ > - if (!strcmp(nat_entry->nb->type, "snat")) { > + if (nat_entry->type == SNAT) { > continue; > } > build_lrouter_nat_arp_nd_flow(od, nat_entry, lflows, meter_groups, > @@ -15544,7 +15544,7 @@ build_lrouter_ipv4_ip_input_for_lbnats( > /* Skip SNAT entries for now, we handle unique SNAT IPs separately > * below. > */ > - if (!strcmp(nat_entry->nb->type, "snat")) { > + if (nat_entry->type == SNAT) { > continue; > } > build_lrouter_port_nat_arp_nd_flow(op, nat_entry, lflows, > @@ -15597,16 +15597,17 @@ build_lrouter_in_unsnat_match(const struct ovn_datapath *od, > static void > build_lrouter_in_unsnat_stateless_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, > + const struct ovn_nat *nat_entry, > struct ds *match, > bool distributed_nat, bool is_v6, > struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref) > { > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > + const struct nbrec_nat *nat = nat_entry->nb; > uint16_t priority = od->is_gw_router ? 90 : 100; > > build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, > @@ -15620,15 +15621,16 @@ build_lrouter_in_unsnat_stateless_flow(struct lflow_table *lflows, > static void > build_lrouter_in_unsnat_in_czone_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, > + const struct ovn_nat *nat_entry, > struct ds *match, bool distributed_nat, > bool is_v6, struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref) > { > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > + const struct nbrec_nat *nat = nat_entry->nb; > build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, > l3dgw_port); > > @@ -15655,16 +15657,16 @@ build_lrouter_in_unsnat_in_czone_flow(struct lflow_table *lflows, > static void > build_lrouter_in_unsnat_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, struct ds *match, > + const struct ovn_nat *nat_entry, struct ds *match, > bool distributed_nat, bool is_v6, > struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref) > { > - > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > + const struct nbrec_nat *nat = nat_entry->nb; > uint16_t priority = od->is_gw_router ? 90 : 100; > > build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, > @@ -15679,7 +15681,7 @@ static void > build_lrouter_in_dnat_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > const struct lr_nat_record *lrnat_rec, > - const struct nbrec_nat *nat, struct ds *match, > + const struct ovn_nat *nat_entry, struct ds *match, > struct ds *actions, bool distributed_nat, > int cidr_bits, bool is_v6, > struct ovn_port *l3dgw_port, bool stateless, > @@ -15688,13 +15690,14 @@ build_lrouter_in_dnat_flow(struct lflow_table *lflows, > /* Ingress DNAT table: Packets enter the pipeline with destination > * IP address that needs to be DNATted from a external IP address > * to a logical IP address. */ > - if (strcmp(nat->type, "dnat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == DNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > ds_clear(match); > ds_clear(actions); > > + const struct nbrec_nat *nat = nat_entry->nb; > const char *nat_action = lrouter_use_common_zone(od) > ? "ct_dnat_in_czone" > : "ct_dnat"; > @@ -15757,11 +15760,11 @@ build_lrouter_in_dnat_flow(struct lflow_table *lflows, > static void > build_lrouter_out_undnat_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, struct ds *match, > - struct ds *actions, bool distributed_nat, > - struct eth_addr mac, bool is_v6, > - struct ovn_port *l3dgw_port, bool stateless, > - struct lflow_ref *lflow_ref) > + const struct ovn_nat *nat_entry, > + struct ds *match, struct ds *actions, > + bool distributed_nat, struct eth_addr mac, > + bool is_v6, struct ovn_port *l3dgw_port, > + bool stateless, struct lflow_ref *lflow_ref) > { > /* Egress UNDNAT table: It is for already established connections' > * reverse traffic. i.e., DNAT has already been done in ingress > @@ -15771,13 +15774,14 @@ build_lrouter_out_undnat_flow(struct lflow_table *lflows, > * Note that this only applies for NAT on a distributed router. > */ > if (!od->n_l3dgw_ports || > - (strcmp(nat->type, "dnat") && strcmp(nat->type, "dnat_and_snat"))) { > + !(nat_entry->type == DNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > ds_clear(match); > ds_clear(actions); > > + const struct nbrec_nat *nat = nat_entry->nb; > ds_put_format(match, "ip && ip%c.src == %s && outport == %s", > is_v6 ? '6' : '4', nat->logical_ip, > l3dgw_port->json_key); > @@ -15883,19 +15887,20 @@ build_lrouter_out_snat_match(struct lflow_table *lflows, > static void > build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, > + const struct ovn_nat *nat_entry, > struct ds *match, struct ds *actions, > bool distributed_nat, > struct eth_addr mac, int cidr_bits, > bool is_v6, struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref) > { > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > ds_clear(actions); > > + const struct nbrec_nat *nat = nat_entry->nb; > uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); > build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, > cidr_bits, is_v6, l3dgw_port, lflow_ref, > @@ -15918,19 +15923,20 @@ build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows, > static void > build_lrouter_out_snat_in_czone_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, > + const struct ovn_nat *nat_entry, > struct ds *match, > struct ds *actions, bool distributed_nat, > struct eth_addr mac, int cidr_bits, > bool is_v6, struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref) > { > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > ds_clear(actions); > > + const struct nbrec_nat *nat = nat_entry->nb; > uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); > struct ds zone_actions = DS_EMPTY_INITIALIZER; > > @@ -15977,19 +15983,20 @@ build_lrouter_out_snat_in_czone_flow(struct lflow_table *lflows, > static void > build_lrouter_out_snat_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, struct ds *match, > + const struct ovn_nat *nat_entry, struct ds *match, > struct ds *actions, bool distributed_nat, > struct eth_addr mac, int cidr_bits, bool is_v6, > struct ovn_port *l3dgw_port, > struct lflow_ref *lflow_ref, > const struct chassis_features *features) > { > - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { > + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { > return; > } > > ds_clear(actions); > > + const struct nbrec_nat *nat = nat_entry->nb; > uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); > > build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, > @@ -16017,7 +16024,7 @@ build_lrouter_out_snat_flow(struct lflow_table *lflows, > * properly tracked so we can decide whether to perform SNAT on traffic > * exiting the network. */ > if (features->ct_commit_to_zone && features->ct_next_zone && > - !strcmp(nat->type, "snat") && !od->is_gw_router) { > + nat_entry->type == SNAT && !od->is_gw_router) { > /* For traffic that comes from SNAT network, initiate CT state before > * entering S_ROUTER_OUT_SNAT to allow matching on various CT states. > */ > @@ -16120,14 +16127,15 @@ build_lrouter_ingress_nat_check_pkt_len(struct lflow_table *lflows, > static void > build_lrouter_ingress_flow(struct lflow_table *lflows, > const struct ovn_datapath *od, > - const struct nbrec_nat *nat, struct ds *match, > + const struct ovn_nat *nat_entry, struct ds *match, > struct ds *actions, struct eth_addr mac, > bool distributed_nat, bool is_v6, > struct ovn_port *l3dgw_port, > const struct shash *meter_groups, > struct lflow_ref *lflow_ref) > { > - if (od->n_l3dgw_ports && !strcmp(nat->type, "snat")) { > + const struct nbrec_nat *nat = nat_entry->nb; > + if (od->n_l3dgw_ports && nat_entry->type == SNAT) { > ds_clear(match); > ds_put_format( > match, "inport == %s && %s == %s", > @@ -16173,11 +16181,12 @@ build_lrouter_ingress_flow(struct lflow_table *lflows, > > static int > lrouter_check_nat_entry(const struct ovn_datapath *od, > - const struct nbrec_nat *nat, > + const struct ovn_nat *nat_entry, > const struct hmap *lr_ports, ovs_be32 *mask, > bool *is_v6, int *cidr_bits, struct eth_addr *mac, > bool *distributed, struct ovn_port **nat_l3dgw_port) > { > + const struct nbrec_nat *nat = nat_entry->nb; > struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT; > ovs_be32 ip; > > @@ -16255,7 +16264,7 @@ lrouter_check_nat_entry(const struct ovn_datapath *od, > error = ip_parse_masked(nat->logical_ip, &ip, mask); > *cidr_bits = ip_count_cidr_bits(*mask); > } > - if (!strcmp(nat->type, "snat")) { > + if (nat_entry->type == SNAT) { > if (error) { > /* Invalid for both IPv4 and IPv6 */ > static struct vlog_rate_limit rl = > @@ -16293,7 +16302,7 @@ lrouter_check_nat_entry(const struct ovn_datapath *od, > return 0; > } > > - if (od->n_l3dgw_ports && !strcmp(nat->type, "dnat_and_snat") && > + if (od->n_l3dgw_ports && nat_entry->type == DNAT_AND_SNAT && > nat->logical_port && nat->external_mac) { > if (eth_addr_from_string(nat->external_mac, mac)) { > *distributed = true; > @@ -16460,9 +16469,9 @@ build_lrouter_nat_defrag_and_lb( > int cidr_bits; > struct ovn_port *l3dgw_port; > > - bool stateless = lrouter_dnat_and_snat_is_stateless(nat); > + bool stateless = lrouter_dnat_and_snat_is_stateless(nat_entry); > > - if (lrouter_check_nat_entry(od, nat, lr_ports, &mask, &is_v6, > + if (lrouter_check_nat_entry(od, nat_entry, lr_ports, &mask, &is_v6, > &cidr_bits, > &mac, &distributed_nat, &l3dgw_port) < 0) { > continue; > @@ -16479,21 +16488,22 @@ build_lrouter_nat_defrag_and_lb( > * not know about the possibility of eventual additional SNAT in > * egress pipeline. */ > if (stateless) { > - build_lrouter_in_unsnat_stateless_flow(lflows, od, nat, match, > - distributed_nat, is_v6, > - l3dgw_port, lflow_ref); > + build_lrouter_in_unsnat_stateless_flow(lflows, od, nat_entry, > + match, distributed_nat, > + is_v6, l3dgw_port, > + lflow_ref); > } else if (lrouter_use_common_zone(od)) { > - build_lrouter_in_unsnat_in_czone_flow(lflows, od, nat, match, > + build_lrouter_in_unsnat_in_czone_flow(lflows, od, nat_entry, match, > distributed_nat, is_v6, > l3dgw_port, lflow_ref); > } else { > - build_lrouter_in_unsnat_flow(lflows, od, nat, match, > + build_lrouter_in_unsnat_flow(lflows, od, nat_entry, match, > distributed_nat, is_v6, l3dgw_port, > lflow_ref); > } > /* S_ROUTER_IN_DNAT */ > - build_lrouter_in_dnat_flow(lflows, od, lrnat_rec, nat, match, actions, > - distributed_nat, cidr_bits, is_v6, > + build_lrouter_in_dnat_flow(lflows, od, lrnat_rec, nat_entry, match, > + actions, distributed_nat, cidr_bits, is_v6, > l3dgw_port, stateless, lflow_ref); > > /* ARP resolve for NAT IPs. */ > @@ -16569,7 +16579,7 @@ build_lrouter_nat_defrag_and_lb( > } > > /* S_ROUTER_OUT_UNDNAT */ > - build_lrouter_out_undnat_flow(lflows, od, nat, match, actions, > + build_lrouter_out_undnat_flow(lflows, od, nat_entry, match, actions, > distributed_nat, mac, is_v6, l3dgw_port, > stateless, lflow_ref); > /* S_ROUTER_OUT_SNAT > @@ -16577,23 +16587,23 @@ build_lrouter_nat_defrag_and_lb( > * source ip address that needs to be SNATted to a external ip > * address. */ > if (stateless) { > - build_lrouter_out_snat_stateless_flow(lflows, od, nat, match, > + build_lrouter_out_snat_stateless_flow(lflows, od, nat_entry, match, > actions, distributed_nat, > mac, cidr_bits, is_v6, > l3dgw_port, lflow_ref); > } else if (lrouter_use_common_zone(od)) { > - build_lrouter_out_snat_in_czone_flow(lflows, od, nat, match, > + build_lrouter_out_snat_in_czone_flow(lflows, od, nat_entry, match, > actions, distributed_nat, mac, > cidr_bits, is_v6, l3dgw_port, > lflow_ref); > } else { > - build_lrouter_out_snat_flow(lflows, od, nat, match, actions, > + build_lrouter_out_snat_flow(lflows, od, nat_entry, match, actions, > distributed_nat, mac, cidr_bits, is_v6, > l3dgw_port, lflow_ref, features); > } > > /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ > - build_lrouter_ingress_flow(lflows, od, nat, match, actions, mac, > + build_lrouter_ingress_flow(lflows, od, nat_entry, match, actions, mac, > distributed_nat, is_v6, l3dgw_port, > meter_groups, lflow_ref); > > -- > 2.46.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c index 215d924e4..bdbb2c860 100644 --- a/northd/en-lr-nat.c +++ b/northd/en-lr-nat.c @@ -313,10 +313,14 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, nat_entry->ext_addrs.ipv6_addrs[0].addr_s, nat_entry); } + nat_entry->type = SNAT; } else { - if (!strcmp(nat->type, "dnat_and_snat") - && nat->logical_port && nat->external_mac) { - lrnat_rec->has_distributed_nat = true; + nat_entry->type = DNAT; + if (!strcmp(nat->type, "dnat_and_snat")) { + nat_entry->type = DNAT_AND_SNAT; + if (nat->logical_port && nat->external_mac) { + lrnat_rec->has_distributed_nat = true; + } } if (nat->external_mac) { diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h index 81a7b0abd..120ceeca9 100644 --- a/northd/en-lr-nat.h +++ b/northd/en-lr-nat.h @@ -29,6 +29,12 @@ #include "lib/ovn-sb-idl.h" #include "lib/ovn-util.h" +enum ovn_nat_type { + SNAT, + DNAT, + DNAT_AND_SNAT, +}; + /* Contains a NAT entry with the external addresses pre-parsed. */ struct ovn_nat { const struct nbrec_nat *nb; @@ -39,6 +45,7 @@ struct ovn_nat { */ bool is_router_ip; /* Indicates if the NAT external_ip is also one of * router's lrp ip. Can be 'true' only for SNAT. */ + enum ovn_nat_type type; }; /* Stores the list of SNAT entries referencing a unique SNAT IP address. diff --git a/northd/northd.c b/northd/northd.c index 2c4703301..0364dd766 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -8950,7 +8950,7 @@ build_lswitch_rport_arp_req_flows_for_lbnats( continue; } - if (!strcmp(nat->type, "snat")) { + if (nat_entry->type == SNAT) { continue; } @@ -10356,8 +10356,8 @@ build_lswitch_ip_unicast_lookup_for_nats( const struct ovn_nat *nat = &lr_stateful_rec->lrnat_rec->nat_entries[i]; - if (!strcmp(nat->nb->type, "dnat_and_snat") - && nat->nb->logical_port && nat->nb->external_mac + if (nat->type == DNAT_AND_SNAT && nat->nb->logical_port + && nat->nb->external_mac && eth_addr_from_string(nat->nb->external_mac, &mac)) { ds_clear(match); @@ -12307,10 +12307,10 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) } static inline bool -lrouter_dnat_and_snat_is_stateless(const struct nbrec_nat *nat) +lrouter_dnat_and_snat_is_stateless(const struct ovn_nat *nat) { - return smap_get_bool(&nat->options, "stateless", false) && - !strcmp(nat->type, "dnat_and_snat"); + return smap_get_bool(&nat->nb->options, "stateless", false) && + nat->type == DNAT_AND_SNAT; } #define NAT_PRIORITY_MATCH_OFFSET 300 @@ -14587,7 +14587,7 @@ build_lr_gateway_redirect_flows_for_nats( for (int j = 0; j < lrnat_rec->n_nat_entries; j++) { const struct ovn_nat *nat = &lrnat_rec->nat_entries[j]; - if (!lrouter_dnat_and_snat_is_stateless(nat->nb) || + if (!lrouter_dnat_and_snat_is_stateless(nat) || (!nat->nb->allowed_ext_ips && !nat->nb->exempted_ext_ips)) { continue; } @@ -15237,7 +15237,7 @@ build_lrouter_arp_nd_for_datapath(const struct ovn_datapath *od, /* Skip SNAT entries for now, we handle unique SNAT IPs separately * below. */ - if (!strcmp(nat_entry->nb->type, "snat")) { + if (nat_entry->type == SNAT) { continue; } build_lrouter_nat_arp_nd_flow(od, nat_entry, lflows, meter_groups, @@ -15544,7 +15544,7 @@ build_lrouter_ipv4_ip_input_for_lbnats( /* Skip SNAT entries for now, we handle unique SNAT IPs separately * below. */ - if (!strcmp(nat_entry->nb->type, "snat")) { + if (nat_entry->type == SNAT) { continue; } build_lrouter_port_nat_arp_nd_flow(op, nat_entry, lflows, @@ -15597,16 +15597,17 @@ build_lrouter_in_unsnat_match(const struct ovn_datapath *od, static void build_lrouter_in_unsnat_stateless_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, + const struct ovn_nat *nat_entry, struct ds *match, bool distributed_nat, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref) { - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } + const struct nbrec_nat *nat = nat_entry->nb; uint16_t priority = od->is_gw_router ? 90 : 100; build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, @@ -15620,15 +15621,16 @@ build_lrouter_in_unsnat_stateless_flow(struct lflow_table *lflows, static void build_lrouter_in_unsnat_in_czone_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, + const struct ovn_nat *nat_entry, struct ds *match, bool distributed_nat, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref) { - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } + const struct nbrec_nat *nat = nat_entry->nb; build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, l3dgw_port); @@ -15655,16 +15657,16 @@ build_lrouter_in_unsnat_in_czone_flow(struct lflow_table *lflows, static void build_lrouter_in_unsnat_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, + const struct ovn_nat *nat_entry, struct ds *match, bool distributed_nat, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref) { - - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } + const struct nbrec_nat *nat = nat_entry->nb; uint16_t priority = od->is_gw_router ? 90 : 100; build_lrouter_in_unsnat_match(od, nat, match, distributed_nat, is_v6, @@ -15679,7 +15681,7 @@ static void build_lrouter_in_dnat_flow(struct lflow_table *lflows, const struct ovn_datapath *od, const struct lr_nat_record *lrnat_rec, - const struct nbrec_nat *nat, struct ds *match, + const struct ovn_nat *nat_entry, struct ds *match, struct ds *actions, bool distributed_nat, int cidr_bits, bool is_v6, struct ovn_port *l3dgw_port, bool stateless, @@ -15688,13 +15690,14 @@ build_lrouter_in_dnat_flow(struct lflow_table *lflows, /* Ingress DNAT table: Packets enter the pipeline with destination * IP address that needs to be DNATted from a external IP address * to a logical IP address. */ - if (strcmp(nat->type, "dnat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == DNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } ds_clear(match); ds_clear(actions); + const struct nbrec_nat *nat = nat_entry->nb; const char *nat_action = lrouter_use_common_zone(od) ? "ct_dnat_in_czone" : "ct_dnat"; @@ -15757,11 +15760,11 @@ build_lrouter_in_dnat_flow(struct lflow_table *lflows, static void build_lrouter_out_undnat_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, - struct ds *actions, bool distributed_nat, - struct eth_addr mac, bool is_v6, - struct ovn_port *l3dgw_port, bool stateless, - struct lflow_ref *lflow_ref) + const struct ovn_nat *nat_entry, + struct ds *match, struct ds *actions, + bool distributed_nat, struct eth_addr mac, + bool is_v6, struct ovn_port *l3dgw_port, + bool stateless, struct lflow_ref *lflow_ref) { /* Egress UNDNAT table: It is for already established connections' * reverse traffic. i.e., DNAT has already been done in ingress @@ -15771,13 +15774,14 @@ build_lrouter_out_undnat_flow(struct lflow_table *lflows, * Note that this only applies for NAT on a distributed router. */ if (!od->n_l3dgw_ports || - (strcmp(nat->type, "dnat") && strcmp(nat->type, "dnat_and_snat"))) { + !(nat_entry->type == DNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } ds_clear(match); ds_clear(actions); + const struct nbrec_nat *nat = nat_entry->nb; ds_put_format(match, "ip && ip%c.src == %s && outport == %s", is_v6 ? '6' : '4', nat->logical_ip, l3dgw_port->json_key); @@ -15883,19 +15887,20 @@ build_lrouter_out_snat_match(struct lflow_table *lflows, static void build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, + const struct ovn_nat *nat_entry, struct ds *match, struct ds *actions, bool distributed_nat, struct eth_addr mac, int cidr_bits, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref) { - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } ds_clear(actions); + const struct nbrec_nat *nat = nat_entry->nb; uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, cidr_bits, is_v6, l3dgw_port, lflow_ref, @@ -15918,19 +15923,20 @@ build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows, static void build_lrouter_out_snat_in_czone_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, + const struct ovn_nat *nat_entry, struct ds *match, struct ds *actions, bool distributed_nat, struct eth_addr mac, int cidr_bits, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref) { - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } ds_clear(actions); + const struct nbrec_nat *nat = nat_entry->nb; uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); struct ds zone_actions = DS_EMPTY_INITIALIZER; @@ -15977,19 +15983,20 @@ build_lrouter_out_snat_in_czone_flow(struct lflow_table *lflows, static void build_lrouter_out_snat_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, + const struct ovn_nat *nat_entry, struct ds *match, struct ds *actions, bool distributed_nat, struct eth_addr mac, int cidr_bits, bool is_v6, struct ovn_port *l3dgw_port, struct lflow_ref *lflow_ref, const struct chassis_features *features) { - if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) { + if (!(nat_entry->type == SNAT || nat_entry->type == DNAT_AND_SNAT)) { return; } ds_clear(actions); + const struct nbrec_nat *nat = nat_entry->nb; uint16_t priority = lrouter_nat_get_priority(od, nat, false, cidr_bits); build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, @@ -16017,7 +16024,7 @@ build_lrouter_out_snat_flow(struct lflow_table *lflows, * properly tracked so we can decide whether to perform SNAT on traffic * exiting the network. */ if (features->ct_commit_to_zone && features->ct_next_zone && - !strcmp(nat->type, "snat") && !od->is_gw_router) { + nat_entry->type == SNAT && !od->is_gw_router) { /* For traffic that comes from SNAT network, initiate CT state before * entering S_ROUTER_OUT_SNAT to allow matching on various CT states. */ @@ -16120,14 +16127,15 @@ build_lrouter_ingress_nat_check_pkt_len(struct lflow_table *lflows, static void build_lrouter_ingress_flow(struct lflow_table *lflows, const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, + const struct ovn_nat *nat_entry, struct ds *match, struct ds *actions, struct eth_addr mac, bool distributed_nat, bool is_v6, struct ovn_port *l3dgw_port, const struct shash *meter_groups, struct lflow_ref *lflow_ref) { - if (od->n_l3dgw_ports && !strcmp(nat->type, "snat")) { + const struct nbrec_nat *nat = nat_entry->nb; + if (od->n_l3dgw_ports && nat_entry->type == SNAT) { ds_clear(match); ds_put_format( match, "inport == %s && %s == %s", @@ -16173,11 +16181,12 @@ build_lrouter_ingress_flow(struct lflow_table *lflows, static int lrouter_check_nat_entry(const struct ovn_datapath *od, - const struct nbrec_nat *nat, + const struct ovn_nat *nat_entry, const struct hmap *lr_ports, ovs_be32 *mask, bool *is_v6, int *cidr_bits, struct eth_addr *mac, bool *distributed, struct ovn_port **nat_l3dgw_port) { + const struct nbrec_nat *nat = nat_entry->nb; struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT; ovs_be32 ip; @@ -16255,7 +16264,7 @@ lrouter_check_nat_entry(const struct ovn_datapath *od, error = ip_parse_masked(nat->logical_ip, &ip, mask); *cidr_bits = ip_count_cidr_bits(*mask); } - if (!strcmp(nat->type, "snat")) { + if (nat_entry->type == SNAT) { if (error) { /* Invalid for both IPv4 and IPv6 */ static struct vlog_rate_limit rl = @@ -16293,7 +16302,7 @@ lrouter_check_nat_entry(const struct ovn_datapath *od, return 0; } - if (od->n_l3dgw_ports && !strcmp(nat->type, "dnat_and_snat") && + if (od->n_l3dgw_ports && nat_entry->type == DNAT_AND_SNAT && nat->logical_port && nat->external_mac) { if (eth_addr_from_string(nat->external_mac, mac)) { *distributed = true; @@ -16460,9 +16469,9 @@ build_lrouter_nat_defrag_and_lb( int cidr_bits; struct ovn_port *l3dgw_port; - bool stateless = lrouter_dnat_and_snat_is_stateless(nat); + bool stateless = lrouter_dnat_and_snat_is_stateless(nat_entry); - if (lrouter_check_nat_entry(od, nat, lr_ports, &mask, &is_v6, + if (lrouter_check_nat_entry(od, nat_entry, lr_ports, &mask, &is_v6, &cidr_bits, &mac, &distributed_nat, &l3dgw_port) < 0) { continue; @@ -16479,21 +16488,22 @@ build_lrouter_nat_defrag_and_lb( * not know about the possibility of eventual additional SNAT in * egress pipeline. */ if (stateless) { - build_lrouter_in_unsnat_stateless_flow(lflows, od, nat, match, - distributed_nat, is_v6, - l3dgw_port, lflow_ref); + build_lrouter_in_unsnat_stateless_flow(lflows, od, nat_entry, + match, distributed_nat, + is_v6, l3dgw_port, + lflow_ref); } else if (lrouter_use_common_zone(od)) { - build_lrouter_in_unsnat_in_czone_flow(lflows, od, nat, match, + build_lrouter_in_unsnat_in_czone_flow(lflows, od, nat_entry, match, distributed_nat, is_v6, l3dgw_port, lflow_ref); } else { - build_lrouter_in_unsnat_flow(lflows, od, nat, match, + build_lrouter_in_unsnat_flow(lflows, od, nat_entry, match, distributed_nat, is_v6, l3dgw_port, lflow_ref); } /* S_ROUTER_IN_DNAT */ - build_lrouter_in_dnat_flow(lflows, od, lrnat_rec, nat, match, actions, - distributed_nat, cidr_bits, is_v6, + build_lrouter_in_dnat_flow(lflows, od, lrnat_rec, nat_entry, match, + actions, distributed_nat, cidr_bits, is_v6, l3dgw_port, stateless, lflow_ref); /* ARP resolve for NAT IPs. */ @@ -16569,7 +16579,7 @@ build_lrouter_nat_defrag_and_lb( } /* S_ROUTER_OUT_UNDNAT */ - build_lrouter_out_undnat_flow(lflows, od, nat, match, actions, + build_lrouter_out_undnat_flow(lflows, od, nat_entry, match, actions, distributed_nat, mac, is_v6, l3dgw_port, stateless, lflow_ref); /* S_ROUTER_OUT_SNAT @@ -16577,23 +16587,23 @@ build_lrouter_nat_defrag_and_lb( * source ip address that needs to be SNATted to a external ip * address. */ if (stateless) { - build_lrouter_out_snat_stateless_flow(lflows, od, nat, match, + build_lrouter_out_snat_stateless_flow(lflows, od, nat_entry, match, actions, distributed_nat, mac, cidr_bits, is_v6, l3dgw_port, lflow_ref); } else if (lrouter_use_common_zone(od)) { - build_lrouter_out_snat_in_czone_flow(lflows, od, nat, match, + build_lrouter_out_snat_in_czone_flow(lflows, od, nat_entry, match, actions, distributed_nat, mac, cidr_bits, is_v6, l3dgw_port, lflow_ref); } else { - build_lrouter_out_snat_flow(lflows, od, nat, match, actions, + build_lrouter_out_snat_flow(lflows, od, nat_entry, match, actions, distributed_nat, mac, cidr_bits, is_v6, l3dgw_port, lflow_ref, features); } /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ - build_lrouter_ingress_flow(lflows, od, nat, match, actions, mac, + build_lrouter_ingress_flow(lflows, od, nat_entry, match, actions, mac, distributed_nat, is_v6, l3dgw_port, meter_groups, lflow_ref);
Most of the functions that are building logical flows for NATs did type comparison via strcmp. Store the type in enum instead in the nat record and compare the enum instead. Signed-off-by: Ales Musil <amusil@redhat.com> --- northd/en-lr-nat.c | 10 +++-- northd/en-lr-nat.h | 7 +++ northd/northd.c | 110 ++++++++++++++++++++++++--------------------- 3 files changed, 74 insertions(+), 53 deletions(-)