| Message ID | 1594213562-22828-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] ovn-northd: Fix is_chassis_resident() match for DNAT. | expand |
On Wed, Jul 8, 2020 at 6:36 PM Dumitru Ceara <dceara@redhat.com> wrote: > While reducing the number of logical flows, a bug was introduced when > building the match for flows that need > "is_chassis_resident(redirect_port)". > This commit removes the wrong additional quotes. > > Fixes: 59340bab9402 ("ovn-northd: Minimize number of ARP/NS responder > flows for DNAT.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > Thanks Dumitru. I applied this patch to master. Numan > --- > northd/ovn-northd.c | 2 +- > tests/ovn-northd.at | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index ddfcebe..f7fd00f 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -8763,7 +8763,7 @@ build_lrouter_flows(struct hmap *datapaths, struct > hmap *ports, > * Also need to avoid generation of multiple ARP responses > * from different chassis. */ > if (op->od->l3redirect_port) { > - ds_put_format(&match, "is_chassis_resident(\"%s\")", > + ds_put_format(&match, "is_chassis_resident(%s)", > op->od->l3redirect_port->json_key); > } > } > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 1e4d20a..7872d50 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1686,10 +1686,10 @@ action=(drop;) > # Priority 92 ARP/NS responders (per distributed gw port), if port is > resident. > AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | > grep "arp\|nd" | sort], [0], [dnl > table=3 (lr_in_ip_input ), priority=92 , dnl > -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && > is_chassis_resident(""cr-lrp-public"")), dnl > +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && > is_chassis_resident("cr-lrp-public")), dnl > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP > reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; > arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;) > table=3 (lr_in_ip_input ), priority=92 , dnl > -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && > is_chassis_resident(""cr-lrp-public"")), dnl > +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && > is_chassis_resident("cr-lrp-public")), dnl > action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP > reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; > arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;) > table=3 (lr_in_ip_input ), priority=92 , dnl > match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && > is_chassis_resident("ls-vm")), dnl > -- > 1.8.3.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 ddfcebe..f7fd00f 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -8763,7 +8763,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * Also need to avoid generation of multiple ARP responses * from different chassis. */ if (op->od->l3redirect_port) { - ds_put_format(&match, "is_chassis_resident(\"%s\")", + ds_put_format(&match, "is_chassis_resident(%s)", op->od->l3redirect_port->json_key); } } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 1e4d20a..7872d50 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1686,10 +1686,10 @@ action=(drop;) # Priority 92 ARP/NS responders (per distributed gw port), if port is resident. AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | grep "arp\|nd" | sort], [0], [dnl table=3 (lr_in_ip_input ), priority=92 , dnl -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident(""cr-lrp-public"")), dnl +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=92 , dnl -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident(""cr-lrp-public"")), dnl +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=92 , dnl match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
While reducing the number of logical flows, a bug was introduced when building the match for flows that need "is_chassis_resident(redirect_port)". This commit removes the wrong additional quotes. Fixes: 59340bab9402 ("ovn-northd: Minimize number of ARP/NS responder flows for DNAT.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- northd/ovn-northd.c | 2 +- tests/ovn-northd.at | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)