Message ID | 20200312102838.1838243-1-numans@ovn.org |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,ovn,v2] ovn-northd: Add lflows to by pass the svc monitor packets from conntrack. | expand |
On 3/12/20 11:28 AM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > The commit [1] added lflows to by pass the service monitor health check > packets from conntrack. But it missed out adding in the ingress pre_acl > and egress pre_acl of logical switch pipeline. > > This patch adds these missing lflows. It also enhanced the system lb health > check tests to add the acls to test this scenario. > > [1] - bb9f2b9ce56c("ovn-northd: Consider load balancer active backends in router pipeline) > Fixes: bb9f2b9ce56c("ovn-northd: Consider load balancer active backends in router pipeline) > > Reported-by: Maciej Józefczyk <mjozefcz@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> Looks good to me. Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru > --- > v1 -> v2 > ----- > * Addressed the review comments from Dumitru > - Added the ACLs in the unit test too. > > northd/ovn-northd.8.xml | 22 +++++++++++++++++++++- > northd/ovn-northd.c | 15 ++++++++++++++- > tests/ovn.at | 22 ++++++++++++++++++++++ > tests/system-ovn.at | 22 ++++++++++++++++++++++ > 4 files changed, 79 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index b6cfa3e90..9b44720d1 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -293,6 +293,16 @@ > priority-110 flow is added to skip over stateful ACLs. > </p> > > + <p> > + This table also has a priority-110 flow with the match > + <code>eth.dst == <var>E</var></code> for all logical switch > + datapaths to move traffic to the next table. Where <var>E</var> > + is the service monitor mac defined in the > + <ref column="options:svc_monitor_mac" table="NB_Global" > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > + db="OVN_Northbound"/> table. > + </p> > + > <h3>Ingress Table 4: Pre-LB</h3> > > <p> > @@ -320,7 +330,7 @@ > > <p> > This table also has a priority-110 flow with the match > - <code>eth.src == <var>E</var></code> for all logical switch > + <code>eth.dst == <var>E</var></code> for all logical switch > datapaths to move traffic to the next table. Where <var>E</var> > is the service monitor mac defined in the > <ref column="options:svc_monitor_mac" table="NB_Global" > @@ -1295,6 +1305,16 @@ output; > <code>to-lport</code> traffic. > </p> > > + <p> > + This table also has a priority-110 flow with the match > + <code>eth.src == <var>E</var></code> for all logical switch > + datapaths to move traffic to the next table. Where <var>E</var> > + is the service monitor mac defined in the > + <ref column="options:svc_monitor_mac" table="NB_Global" > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > + db="OVN_Northbound"/> table. > + </p> > + > <h3>Egress Table 2: Pre-stateful</h3> > > <p> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index d42a9892a..52dca48c6 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -4605,6 +4605,16 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;"); > > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match, > + "next;"); > + free(svc_check_match); > + > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, svc_check_match, > + "next;"); > + free(svc_check_match); > + > /* If there are any stateful ACL rules in this datapath, we must > * send all IP packets through the conntrack action, which handles > * defragmentation, in order to match L4 headers. */ > @@ -4788,9 +4798,12 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, > "next;"); > > /* Do not send service monitor packets to conntrack. */ > - char *svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, > svc_check_match, "next;"); > + free(svc_check_match); > + > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, > svc_check_match, "next;"); > free(svc_check_match); > diff --git a/tests/ovn.at b/tests/ovn.at > index 8de4b5ceb..8cdbad743 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -17739,12 +17739,34 @@ ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3" > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > +# Create port group and ACLs for sw0 ports. > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop > + > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > + > # Create the second logical switch with one port > ovn-nbctl ls-add sw1 > ovn-nbctl lsp-add sw1 sw1-p1 > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > +# Create port group and ACLs for sw1 ports. > +ovn-nbctl pg-add pg1_drop sw1-p1 > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" drop > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" drop > + > +ovn-nbctl pg-add pg1 sw1-p1 > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > + > # Create a logical router and attach both logical switches > ovn-nbctl lr-add lr0 > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 9ed3df754..3b3379840 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -3314,12 +3314,34 @@ ovn-nbctl lsp-add sw0 sw0-p2 > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > +# Create port group and ACLs for sw0 ports. > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop > + > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > + > # Create the second logical switch with one port > ovn-nbctl ls-add sw1 > ovn-nbctl lsp-add sw1 sw1-p1 > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > +# Create port group and ACLs for sw1 ports. > +ovn-nbctl pg-add pg1_drop sw1-p1 > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" drop > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" drop > + > +ovn-nbctl pg-add pg1 sw1-p1 > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > + > # Create a logical router and attach both logical switches > ovn-nbctl lr-add lr0 > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >
Looks good. I tested it on my env. The HC mechanism works fine. I also verified it for more than one VIPs configured. Acked-by: Maciej Jozefczyk <mjozefcz@redhat.com <dceara@redhat.com>> Thanks, Maciej On Thu, Mar 12, 2020 at 1:39 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 3/12/20 11:28 AM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > The commit [1] added lflows to by pass the service monitor health check > > packets from conntrack. But it missed out adding in the ingress pre_acl > > and egress pre_acl of logical switch pipeline. > > > > This patch adds these missing lflows. It also enhanced the system lb > health > > check tests to add the acls to test this scenario. > > > > [1] - bb9f2b9ce56c("ovn-northd: Consider load balancer active backends > in router pipeline) > > Fixes: bb9f2b9ce56c("ovn-northd: Consider load balancer active backends > in router pipeline) > > > > Reported-by: Maciej Józefczyk <mjozefcz@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > Looks good to me. > > Acked-by: Dumitru Ceara <dceara@redhat.com> > > Thanks, > Dumitru > > > --- > > v1 -> v2 > > ----- > > * Addressed the review comments from Dumitru > > - Added the ACLs in the unit test too. > > > > northd/ovn-northd.8.xml | 22 +++++++++++++++++++++- > > northd/ovn-northd.c | 15 ++++++++++++++- > > tests/ovn.at | 22 ++++++++++++++++++++++ > > tests/system-ovn.at | 22 ++++++++++++++++++++++ > > 4 files changed, 79 insertions(+), 2 deletions(-) > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > index b6cfa3e90..9b44720d1 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -293,6 +293,16 @@ > > priority-110 flow is added to skip over stateful ACLs. > > </p> > > > > + <p> > > + This table also has a priority-110 flow with the match > > + <code>eth.dst == <var>E</var></code> for all logical switch > > + datapaths to move traffic to the next table. Where <var>E</var> > > + is the service monitor mac defined in the > > + <ref column="options:svc_monitor_mac" table="NB_Global" > > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > > + db="OVN_Northbound"/> table. > > + </p> > > + > > <h3>Ingress Table 4: Pre-LB</h3> > > > > <p> > > @@ -320,7 +330,7 @@ > > > > <p> > > This table also has a priority-110 flow with the match > > - <code>eth.src == <var>E</var></code> for all logical switch > > + <code>eth.dst == <var>E</var></code> for all logical switch > > datapaths to move traffic to the next table. Where <var>E</var> > > is the service monitor mac defined in the > > <ref column="options:svc_monitor_mac" table="NB_Global" > > @@ -1295,6 +1305,16 @@ output; > > <code>to-lport</code> traffic. > > </p> > > > > + <p> > > + This table also has a priority-110 flow with the match > > + <code>eth.src == <var>E</var></code> for all logical switch > > + datapaths to move traffic to the next table. Where <var>E</var> > > + is the service monitor mac defined in the > > + <ref column="options:svc_monitor_mac" table="NB_Global" > > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > > + db="OVN_Northbound"/> table. > > + </p> > > + > > <h3>Egress Table 2: Pre-stateful</h3> > > > > <p> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index d42a9892a..52dca48c6 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -4605,6 +4605,16 @@ build_pre_acls(struct ovn_datapath *od, struct > hmap *lflows) > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;"); > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;"); > > > > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match, > > + "next;"); > > + free(svc_check_match); > > + > > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, > svc_check_match, > > + "next;"); > > + free(svc_check_match); > > + > > /* If there are any stateful ACL rules in this datapath, we must > > * send all IP packets through the conntrack action, which handles > > * defragmentation, in order to match L4 headers. */ > > @@ -4788,9 +4798,12 @@ build_pre_lb(struct ovn_datapath *od, struct hmap > *lflows, > > "next;"); > > > > /* Do not send service monitor packets to conntrack. */ > > - char *svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, > > svc_check_match, "next;"); > > + free(svc_check_match); > > + > > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, > > svc_check_match, "next;"); > > free(svc_check_match); > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 8de4b5ceb..8cdbad743 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -17739,12 +17739,34 @@ ovn-nbctl lsp-set-port-security sw0-p1 > "50:54:00:00:00:03 10.0.0.3" > > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > > +# Create port group and ACLs for sw0 ports. > > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" > drop > > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" > drop > > + > > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" > allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && icmp4" allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > + > > # Create the second logical switch with one port > > ovn-nbctl ls-add sw1 > > ovn-nbctl lsp-add sw1 sw1-p1 > > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > > +# Create port group and ACLs for sw1 ports. > > +ovn-nbctl pg-add pg1_drop sw1-p1 > > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" > drop > > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" > drop > > + > > +ovn-nbctl pg-add pg1 sw1-p1 > > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" > allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && icmp4" allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > + > > # Create a logical router and attach both logical switches > > ovn-nbctl lr-add lr0 > > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 9ed3df754..3b3379840 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -3314,12 +3314,34 @@ ovn-nbctl lsp-add sw0 sw0-p2 > > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > > +# Create port group and ACLs for sw0 ports. > > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" > drop > > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" > drop > > + > > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" > allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && icmp4" allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > + > > # Create the second logical switch with one port > > ovn-nbctl ls-add sw1 > > ovn-nbctl lsp-add sw1 sw1-p1 > > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > > +# Create port group and ACLs for sw1 ports. > > +ovn-nbctl pg-add pg1_drop sw1-p1 > > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" > drop > > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" > drop > > + > > +ovn-nbctl pg-add pg1 sw1-p1 > > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" > allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && icmp4" allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > + > > # Create a logical router and attach both logical switches > > ovn-nbctl lr-add lr0 > > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, Mar 12, 2020 at 6:20 PM Maciej Jozefczyk <mjozefcz@redhat.com> wrote: > > Looks good. I tested it on my env. The HC mechanism works fine. I also > verified it for more than one VIPs configured. > > Acked-by: Maciej Jozefczyk <mjozefcz@redhat.com <dceara@redhat.com>> Thanks Dumitru and Maciej for the reviews and testing. I applied this patch to master and branch-20.03 Thanks Numan > > Thanks, > Maciej > > On Thu, Mar 12, 2020 at 1:39 PM Dumitru Ceara <dceara@redhat.com> wrote: > > > On 3/12/20 11:28 AM, numans@ovn.org wrote: > > > From: Numan Siddique <numans@ovn.org> > > > > > > The commit [1] added lflows to by pass the service monitor health check > > > packets from conntrack. But it missed out adding in the ingress pre_acl > > > and egress pre_acl of logical switch pipeline. > > > > > > This patch adds these missing lflows. It also enhanced the system lb > > health > > > check tests to add the acls to test this scenario. > > > > > > [1] - bb9f2b9ce56c("ovn-northd: Consider load balancer active backends > > in router pipeline) > > > Fixes: bb9f2b9ce56c("ovn-northd: Consider load balancer active backends > > in router pipeline) > > > > > > Reported-by: Maciej Józefczyk <mjozefcz@redhat.com> > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > > > Looks good to me. > > > > Acked-by: Dumitru Ceara <dceara@redhat.com> > > > > Thanks, > > Dumitru > > > > > --- > > > v1 -> v2 > > > ----- > > > * Addressed the review comments from Dumitru > > > - Added the ACLs in the unit test too. > > > > > > northd/ovn-northd.8.xml | 22 +++++++++++++++++++++- > > > northd/ovn-northd.c | 15 ++++++++++++++- > > > tests/ovn.at | 22 ++++++++++++++++++++++ > > > tests/system-ovn.at | 22 ++++++++++++++++++++++ > > > 4 files changed, 79 insertions(+), 2 deletions(-) > > > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > > index b6cfa3e90..9b44720d1 100644 > > > --- a/northd/ovn-northd.8.xml > > > +++ b/northd/ovn-northd.8.xml > > > @@ -293,6 +293,16 @@ > > > priority-110 flow is added to skip over stateful ACLs. > > > </p> > > > > > > + <p> > > > + This table also has a priority-110 flow with the match > > > + <code>eth.dst == <var>E</var></code> for all logical switch > > > + datapaths to move traffic to the next table. Where <var>E</var> > > > + is the service monitor mac defined in the > > > + <ref column="options:svc_monitor_mac" table="NB_Global" > > > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > > > + db="OVN_Northbound"/> table. > > > + </p> > > > + > > > <h3>Ingress Table 4: Pre-LB</h3> > > > > > > <p> > > > @@ -320,7 +330,7 @@ > > > > > > <p> > > > This table also has a priority-110 flow with the match > > > - <code>eth.src == <var>E</var></code> for all logical switch > > > + <code>eth.dst == <var>E</var></code> for all logical switch > > > datapaths to move traffic to the next table. Where <var>E</var> > > > is the service monitor mac defined in the > > > <ref column="options:svc_monitor_mac" table="NB_Global" > > > @@ -1295,6 +1305,16 @@ output; > > > <code>to-lport</code> traffic. > > > </p> > > > > > > + <p> > > > + This table also has a priority-110 flow with the match > > > + <code>eth.src == <var>E</var></code> for all logical switch > > > + datapaths to move traffic to the next table. Where <var>E</var> > > > + is the service monitor mac defined in the > > > + <ref column="options:svc_monitor_mac" table="NB_Global" > > > + db="OVN_Northbound"/> colum of <ref table="NB_Global" > > > + db="OVN_Northbound"/> table. > > > + </p> > > > + > > > <h3>Egress Table 2: Pre-stateful</h3> > > > > > > <p> > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > > index d42a9892a..52dca48c6 100644 > > > --- a/northd/ovn-northd.c > > > +++ b/northd/ovn-northd.c > > > @@ -4605,6 +4605,16 @@ build_pre_acls(struct ovn_datapath *od, struct > > hmap *lflows) > > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;"); > > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;"); > > > > > > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match, > > > + "next;"); > > > + free(svc_check_match); > > > + > > > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, > > svc_check_match, > > > + "next;"); > > > + free(svc_check_match); > > > + > > > /* If there are any stateful ACL rules in this datapath, we must > > > * send all IP packets through the conntrack action, which handles > > > * defragmentation, in order to match L4 headers. */ > > > @@ -4788,9 +4798,12 @@ build_pre_lb(struct ovn_datapath *od, struct hmap > > *lflows, > > > "next;"); > > > > > > /* Do not send service monitor packets to conntrack. */ > > > - char *svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > > + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, > > > svc_check_match, "next;"); > > > + free(svc_check_match); > > > + > > > + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); > > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, > > > svc_check_match, "next;"); > > > free(svc_check_match); > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index 8de4b5ceb..8cdbad743 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -17739,12 +17739,34 @@ ovn-nbctl lsp-set-port-security sw0-p1 > > "50:54:00:00:00:03 10.0.0.3" > > > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > > > > +# Create port group and ACLs for sw0 ports. > > > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > > > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" > > drop > > > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" > > drop > > > + > > > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > > > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" > > allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && icmp4" allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > > + > > > # Create the second logical switch with one port > > > ovn-nbctl ls-add sw1 > > > ovn-nbctl lsp-add sw1 sw1-p1 > > > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > > > > +# Create port group and ACLs for sw1 ports. > > > +ovn-nbctl pg-add pg1_drop sw1-p1 > > > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" > > drop > > > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" > > drop > > > + > > > +ovn-nbctl pg-add pg1 sw1-p1 > > > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" > > allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && icmp4" allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > > + > > > # Create a logical router and attach both logical switches > > > ovn-nbctl lr-add lr0 > > > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > > index 9ed3df754..3b3379840 100644 > > > --- a/tests/system-ovn.at > > > +++ b/tests/system-ovn.at > > > @@ -3314,12 +3314,34 @@ ovn-nbctl lsp-add sw0 sw0-p2 > > > ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" > > > > > > +# Create port group and ACLs for sw0 ports. > > > +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 > > > +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" > > drop > > > +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" > > drop > > > + > > > +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 > > > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" > > allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && icmp4" allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > > +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > > + > > > # Create the second logical switch with one port > > > ovn-nbctl ls-add sw1 > > > ovn-nbctl lsp-add sw1 sw1-p1 > > > ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" > > > > > > +# Create port group and ACLs for sw1 ports. > > > +ovn-nbctl pg-add pg1_drop sw1-p1 > > > +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" > > drop > > > +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" > > drop > > > + > > > +ovn-nbctl pg-add pg1 sw1-p1 > > > +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" > > allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && icmp4" allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related > > > +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src > > == 0.0.0.0/0 && udp && udp.dst == 80" allow-related > > > + > > > # Create a logical router and attach both logical switches > > > ovn-nbctl lr-add lr0 > > > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > -- > Best regards, > Maciej Józefczyk > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index b6cfa3e90..9b44720d1 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -293,6 +293,16 @@ priority-110 flow is added to skip over stateful ACLs. </p> + <p> + This table also has a priority-110 flow with the match + <code>eth.dst == <var>E</var></code> for all logical switch + datapaths to move traffic to the next table. Where <var>E</var> + is the service monitor mac defined in the + <ref column="options:svc_monitor_mac" table="NB_Global" + db="OVN_Northbound"/> colum of <ref table="NB_Global" + db="OVN_Northbound"/> table. + </p> + <h3>Ingress Table 4: Pre-LB</h3> <p> @@ -320,7 +330,7 @@ <p> This table also has a priority-110 flow with the match - <code>eth.src == <var>E</var></code> for all logical switch + <code>eth.dst == <var>E</var></code> for all logical switch datapaths to move traffic to the next table. Where <var>E</var> is the service monitor mac defined in the <ref column="options:svc_monitor_mac" table="NB_Global" @@ -1295,6 +1305,16 @@ output; <code>to-lport</code> traffic. </p> + <p> + This table also has a priority-110 flow with the match + <code>eth.src == <var>E</var></code> for all logical switch + datapaths to move traffic to the next table. Where <var>E</var> + is the service monitor mac defined in the + <ref column="options:svc_monitor_mac" table="NB_Global" + db="OVN_Northbound"/> colum of <ref table="NB_Global" + db="OVN_Northbound"/> table. + </p> + <h3>Egress Table 2: Pre-stateful</h3> <p> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d42a9892a..52dca48c6 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4605,6 +4605,16 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;"); + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match, + "next;"); + free(svc_check_match); + + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, svc_check_match, + "next;"); + free(svc_check_match); + /* If there are any stateful ACL rules in this datapath, we must * send all IP packets through the conntrack action, which handles * defragmentation, in order to match L4 headers. */ @@ -4788,9 +4798,12 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, "next;"); /* Do not send service monitor packets to conntrack. */ - char *svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); + char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, svc_check_match, "next;"); + free(svc_check_match); + + svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, svc_check_match, "next;"); free(svc_check_match); diff --git a/tests/ovn.at b/tests/ovn.at index 8de4b5ceb..8cdbad743 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -17739,12 +17739,34 @@ ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3" ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" +# Create port group and ACLs for sw0 ports. +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop + +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related + # Create the second logical switch with one port ovn-nbctl ls-add sw1 ovn-nbctl lsp-add sw1 sw1-p1 ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" +# Create port group and ACLs for sw1 ports. +ovn-nbctl pg-add pg1_drop sw1-p1 +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" drop +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" drop + +ovn-nbctl pg-add pg1 sw1-p1 +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related + # Create a logical router and attach both logical switches ovn-nbctl lr-add lr0 ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 9ed3df754..3b3379840 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -3314,12 +3314,34 @@ ovn-nbctl lsp-add sw0 sw0-p2 ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4" ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4" +# Create port group and ACLs for sw0 ports. +ovn-nbctl pg-add pg0_drop sw0-p1 sw0-p2 +ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop +ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop + +ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related +ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related + # Create the second logical switch with one port ovn-nbctl ls-add sw1 ovn-nbctl lsp-add sw1 sw1-p1 ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3" ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3" +# Create port group and ACLs for sw1 ports. +ovn-nbctl pg-add pg1_drop sw1-p1 +ovn-nbctl acl-add pg1_drop from-lport 1001 "inport == @pg1_drop && ip" drop +ovn-nbctl acl-add pg1_drop to-lport 1001 "outport == @pg1_drop && ip" drop + +ovn-nbctl pg-add pg1 sw1-p1 +ovn-nbctl acl-add pg1 from-lport 1002 "inport == @pg1 && ip4" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 80" allow-related +ovn-nbctl acl-add pg1 to-lport 1002 "outport == @pg1 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 80" allow-related + # Create a logical router and attach both logical switches ovn-nbctl lr-add lr0 ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24