| Message ID | 0b560179d108f3ef9f04adfb715ca6f2126ae5f2.1583944844.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] northd: do not insert identical lflows in S_ROUTER_IN_ARP_RESOLVE | expand |
On Wed, Mar 11, 2020 at 10:12 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > Avoid to configure multiple identical logical flows in > S_ROUTER_IN_ARP_RESOLVE stage. This can happen adding L2 destination > address info about snat since multiple nat entries will use the same > external_ip > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Thanks Lorenzo for the fix. I applied this patch to master. Numan > --- > northd/ovn-northd.c | 36 ++++++++++++++++++++++-------------- > 1 file changed, 22 insertions(+), 14 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index d42a9892a..921fe1865 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -8614,6 +8614,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > continue; > } > > + struct sset nat_entries = SSET_INITIALIZER(&nat_entries); > + > struct v46_ip snat_ip, lb_snat_ip; > const char *dnat_force_snat_ip = get_force_snat_ip(od, "dnat", > &snat_ip); > @@ -8839,20 +8841,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > &nat->header_); > } > > - ds_clear(&match); > - ds_put_format( > - &match, "outport == %s && %s == %s", > - od->l3dgw_port->json_key, > - is_v6 ? "xxreg0" : "reg0", nat->external_ip); > - ds_clear(&actions); > - ds_put_format( > - &actions, "eth.dst = %s; next;", > - distributed ? nat->external_mac : > - od->l3dgw_port->lrp_networks.ea_s); > - ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE, > - 100, ds_cstr(&match), > - ds_cstr(&actions), > - &nat->header_); > + if (!sset_contains(&nat_entries, nat->external_ip)) { > + ds_clear(&match); > + ds_put_format( > + &match, "outport == %s && %s == %s", > + od->l3dgw_port->json_key, > + is_v6 ? "xxreg0" : "reg0", nat->external_ip); > + ds_clear(&actions); > + ds_put_format( > + &actions, "eth.dst = %s; next;", > + distributed ? nat->external_mac : > + od->l3dgw_port->lrp_networks.ea_s); > + ovn_lflow_add_with_hint(lflows, od, > + S_ROUTER_IN_ARP_RESOLVE, > + 100, ds_cstr(&match), > + ds_cstr(&actions), > + &nat->header_); > + sset_add(&nat_entries, nat->external_ip); > + } > } > > /* Egress UNDNAT table: It is for already established connections' > @@ -9033,6 +9039,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > } > } > > + sset_destroy(&nat_entries); > + > /* Handle force SNAT options set in the gateway router. */ > if (dnat_force_snat_ip && !od->l3dgw_port) { > /* If a packet with destination IP address as that of the > -- > 2.24.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d42a9892a..921fe1865 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -8614,6 +8614,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, continue; } + struct sset nat_entries = SSET_INITIALIZER(&nat_entries); + struct v46_ip snat_ip, lb_snat_ip; const char *dnat_force_snat_ip = get_force_snat_ip(od, "dnat", &snat_ip); @@ -8839,20 +8841,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, &nat->header_); } - ds_clear(&match); - ds_put_format( - &match, "outport == %s && %s == %s", - od->l3dgw_port->json_key, - is_v6 ? "xxreg0" : "reg0", nat->external_ip); - ds_clear(&actions); - ds_put_format( - &actions, "eth.dst = %s; next;", - distributed ? nat->external_mac : - od->l3dgw_port->lrp_networks.ea_s); - ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE, - 100, ds_cstr(&match), - ds_cstr(&actions), - &nat->header_); + if (!sset_contains(&nat_entries, nat->external_ip)) { + ds_clear(&match); + ds_put_format( + &match, "outport == %s && %s == %s", + od->l3dgw_port->json_key, + is_v6 ? "xxreg0" : "reg0", nat->external_ip); + ds_clear(&actions); + ds_put_format( + &actions, "eth.dst = %s; next;", + distributed ? nat->external_mac : + od->l3dgw_port->lrp_networks.ea_s); + ovn_lflow_add_with_hint(lflows, od, + S_ROUTER_IN_ARP_RESOLVE, + 100, ds_cstr(&match), + ds_cstr(&actions), + &nat->header_); + sset_add(&nat_entries, nat->external_ip); + } } /* Egress UNDNAT table: It is for already established connections' @@ -9033,6 +9039,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } } + sset_destroy(&nat_entries); + /* Handle force SNAT options set in the gateway router. */ if (dnat_force_snat_ip && !od->l3dgw_port) { /* If a packet with destination IP address as that of the
Avoid to configure multiple identical logical flows in S_ROUTER_IN_ARP_RESOLVE stage. This can happen adding L2 destination address info about snat since multiple nat entries will use the same external_ip Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- northd/ovn-northd.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-)