diff mbox series

[ovs-dev] ovn-northd-ddlog: Fix weight overflows.

Message ID 20210514042026.2799046-1-blp@ovn.org
State Accepted
Headers show
Series [ovs-dev] ovn-northd-ddlog: Fix weight overflows. | expand

Commit Message

Ben Pfaff May 14, 2021, 4:20 a.m. UTC
This fixes test failures in "ovn -- ACL allow-stateless omit conntrack -
Port_Group -- ovn-northd-ddlog" with and without "dp-groups=yes".  See
https://github.com/vmware/differential-datalog/pull/977 for the full
story.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Reported-by: Ihar Hrachyshka <ihrachys@redhat.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382967.html
Suggested-by: Leonid Ryhzyk <lryzhyk@vmware.com>
Suggested-by: Mihai Budiu <mbudiu@vmware.com>
---
 northd/lswitch.dl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Numan Siddique May 14, 2021, 11:08 a.m. UTC | #1
On Fri, May 14, 2021 at 12:21 AM Ben Pfaff <blp@ovn.org> wrote:
>
> This fixes test failures in "ovn -- ACL allow-stateless omit conntrack -
> Port_Group -- ovn-northd-ddlog" with and without "dp-groups=yes".  See
> https://github.com/vmware/differential-datalog/pull/977 for the full
> story.
>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> Reported-by: Ihar Hrachyshka <ihrachys@redhat.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382967.html
> Suggested-by: Leonid Ryhzyk <lryzhyk@vmware.com>
> Suggested-by: Mihai Budiu <mbudiu@vmware.com>

Thanks for fixing this.

Should we need to document it somewhere in OVN to use "output relation" ?

Acked-by: Numan Siddique <numans@ovn.org>

Numan


> ---
>  northd/lswitch.dl | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/northd/lswitch.dl b/northd/lswitch.dl
> index 8b1f35ac4378..8fbb313b9666 100644
> --- a/northd/lswitch.dl
> +++ b/northd/lswitch.dl
> @@ -70,7 +70,7 @@ LogicalSwitchPortWithUnknownAddress(ls_uuid, lsp_uuid) :-
>      lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
>      lsp.is_enabled() and lsp.addresses.contains("unknown").
>
> -relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
> +output relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
>  LogicalSwitchHasUnknownPorts(ls, true) :- LogicalSwitchPortWithUnknownAddress(ls, _).
>  LogicalSwitchHasUnknownPorts(ls, false) :-
>      nb::Logical_Switch(._uuid = ls),
> @@ -116,7 +116,7 @@ LogicalSwitchStatefulACL(ls, acl) :-
>      LogicalSwitchACL(ls, acl),
>      nb::ACL(._uuid = acl, .action = "allow-related").
>
> -relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
> +output relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
>
>  LogicalSwitchHasStatefulACL(ls, true) :-
>      LogicalSwitchStatefulACL(ls, _).
> @@ -125,7 +125,7 @@ LogicalSwitchHasStatefulACL(ls, false) :-
>      nb::Logical_Switch(._uuid = ls),
>      not LogicalSwitchStatefulACL(ls, _).
>
> -relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
> +output relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
>
>  LogicalSwitchHasACLs(ls, true) :-
>      LogicalSwitchACL(ls, _).
> @@ -170,7 +170,7 @@ LogicalSwitchWithDNSRecords(ls) :-
>      nb::DNS(._uuid = dns_uuid, .records = records),
>      not records.is_empty().
>
> -relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records: bool)
> +output relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records: bool)
>
>  LogicalSwitchHasDNSRecords(ls, true) :-
>      LogicalSwitchWithDNSRecords(ls).
> @@ -186,7 +186,7 @@ LogicalSwitchHasNonRouterPort0(ls_uuid) :-
>      lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
>      lsp.__type != "router".
>
> -relation LogicalSwitchHasNonRouterPort(ls: uuid, has_non_router_port: bool)
> +output relation LogicalSwitchHasNonRouterPort(ls: uuid, has_non_router_port: bool)
>  LogicalSwitchHasNonRouterPort(ls, true) :-
>      LogicalSwitchHasNonRouterPort0(ls).
>  LogicalSwitchHasNonRouterPort(ls, false) :-
> @@ -285,7 +285,7 @@ SwitchLBVIP(sw_uuid, lb, vip, backends) :-
>      var kv = FlatMap(vips),
>      (var vip, var backends) = kv.
>
> -relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
> +output relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
>  LogicalSwitchHasLBVIP(sw_uuid, true) :-
>      SwitchLBVIP(.sw_uuid = sw_uuid).
>  LogicalSwitchHasLBVIP(sw_uuid, false) :-
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Han Zhou May 14, 2021, 8:37 p.m. UTC | #2
On Thu, May 13, 2021 at 9:20 PM Ben Pfaff <blp@ovn.org> wrote:
>
> This fixes test failures in "ovn -- ACL allow-stateless omit conntrack -
> Port_Group -- ovn-northd-ddlog" with and without "dp-groups=yes".  See
> https://github.com/vmware/differential-datalog/pull/977 for the full
> story.
>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> Reported-by: Ihar Hrachyshka <ihrachys@redhat.com>
> Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382967.html
> Suggested-by: Leonid Ryhzyk <lryzhyk@vmware.com>
> Suggested-by: Mihai Budiu <mbudiu@vmware.com>
> ---
>  northd/lswitch.dl | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/northd/lswitch.dl b/northd/lswitch.dl
> index 8b1f35ac4378..8fbb313b9666 100644
> --- a/northd/lswitch.dl
> +++ b/northd/lswitch.dl
> @@ -70,7 +70,7 @@ LogicalSwitchPortWithUnknownAddress(ls_uuid, lsp_uuid)
:-
>      lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
>      lsp.is_enabled() and lsp.addresses.contains("unknown").
>
> -relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
> +output relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
>  LogicalSwitchHasUnknownPorts(ls, true) :-
LogicalSwitchPortWithUnknownAddress(ls, _).
>  LogicalSwitchHasUnknownPorts(ls, false) :-
>      nb::Logical_Switch(._uuid = ls),
> @@ -116,7 +116,7 @@ LogicalSwitchStatefulACL(ls, acl) :-
>      LogicalSwitchACL(ls, acl),
>      nb::ACL(._uuid = acl, .action = "allow-related").
>
> -relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
> +output relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl:
bool)
>
>  LogicalSwitchHasStatefulACL(ls, true) :-
>      LogicalSwitchStatefulACL(ls, _).
> @@ -125,7 +125,7 @@ LogicalSwitchHasStatefulACL(ls, false) :-
>      nb::Logical_Switch(._uuid = ls),
>      not LogicalSwitchStatefulACL(ls, _).
>
> -relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
> +output relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
>
>  LogicalSwitchHasACLs(ls, true) :-
>      LogicalSwitchACL(ls, _).
> @@ -170,7 +170,7 @@ LogicalSwitchWithDNSRecords(ls) :-
>      nb::DNS(._uuid = dns_uuid, .records = records),
>      not records.is_empty().
>
> -relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records: bool)
> +output relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records:
bool)
>
>  LogicalSwitchHasDNSRecords(ls, true) :-
>      LogicalSwitchWithDNSRecords(ls).
> @@ -186,7 +186,7 @@ LogicalSwitchHasNonRouterPort0(ls_uuid) :-
>      lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
>      lsp.__type != "router".
>
> -relation LogicalSwitchHasNonRouterPort(ls: uuid, has_non_router_port:
bool)
> +output relation LogicalSwitchHasNonRouterPort(ls: uuid,
has_non_router_port: bool)
>  LogicalSwitchHasNonRouterPort(ls, true) :-
>      LogicalSwitchHasNonRouterPort0(ls).
>  LogicalSwitchHasNonRouterPort(ls, false) :-
> @@ -285,7 +285,7 @@ SwitchLBVIP(sw_uuid, lb, vip, backends) :-
>      var kv = FlatMap(vips),
>      (var vip, var backends) = kv.
>
> -relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
> +output relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
>  LogicalSwitchHasLBVIP(sw_uuid, true) :-
>      SwitchLBVIP(.sw_uuid = sw_uuid).
>  LogicalSwitchHasLBVIP(sw_uuid, false) :-
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Ben! (Great explanation in the ddlog doc)

Acked-by: Han Zhou <hzhou@ovn.org>
Ben Pfaff May 14, 2021, 8:53 p.m. UTC | #3
On Fri, May 14, 2021 at 07:08:16AM -0400, Numan Siddique wrote:
> On Fri, May 14, 2021 at 12:21 AM Ben Pfaff <blp@ovn.org> wrote:
> >
> > This fixes test failures in "ovn -- ACL allow-stateless omit conntrack -
> > Port_Group -- ovn-northd-ddlog" with and without "dp-groups=yes".  See
> > https://github.com/vmware/differential-datalog/pull/977 for the full
> > story.
> >
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > Reported-by: Ihar Hrachyshka <ihrachys@redhat.com>
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382967.html
> > Suggested-by: Leonid Ryhzyk <lryzhyk@vmware.com>
> > Suggested-by: Mihai Budiu <mbudiu@vmware.com>
> 
> Thanks for fixing this.

Thanks, I applied it to ovn master.

> Should we need to document it somewhere in OVN to use "output relation" ?

Yes, I was somewhat lazy.  Now sent as followup patch:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/383027.html
Ben Pfaff May 14, 2021, 8:54 p.m. UTC | #4
On Fri, May 14, 2021 at 01:37:08PM -0700, Han Zhou wrote:
> On Thu, May 13, 2021 at 9:20 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > This fixes test failures in "ovn -- ACL allow-stateless omit conntrack -
> > Port_Group -- ovn-northd-ddlog" with and without "dp-groups=yes".  See
> > https://github.com/vmware/differential-datalog/pull/977 for the full
> > story.
> >
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > Reported-by: Ihar Hrachyshka <ihrachys@redhat.com>
> > Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382967.html
> > Suggested-by: Leonid Ryhzyk <lryzhyk@vmware.com>
> > Suggested-by: Mihai Budiu <mbudiu@vmware.com>

...

> Thanks Ben! (Great explanation in the ddlog doc)
> 
> Acked-by: Han Zhou <hzhou@ovn.org>

Thanks so much for the review!
diff mbox series

Patch

diff --git a/northd/lswitch.dl b/northd/lswitch.dl
index 8b1f35ac4378..8fbb313b9666 100644
--- a/northd/lswitch.dl
+++ b/northd/lswitch.dl
@@ -70,7 +70,7 @@  LogicalSwitchPortWithUnknownAddress(ls_uuid, lsp_uuid) :-
     lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
     lsp.is_enabled() and lsp.addresses.contains("unknown").
 
-relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
+output relation LogicalSwitchHasUnknownPorts(ls: uuid, has_unknown: bool)
 LogicalSwitchHasUnknownPorts(ls, true) :- LogicalSwitchPortWithUnknownAddress(ls, _).
 LogicalSwitchHasUnknownPorts(ls, false) :-
     nb::Logical_Switch(._uuid = ls),
@@ -116,7 +116,7 @@  LogicalSwitchStatefulACL(ls, acl) :-
     LogicalSwitchACL(ls, acl),
     nb::ACL(._uuid = acl, .action = "allow-related").
 
-relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
+output relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
 
 LogicalSwitchHasStatefulACL(ls, true) :-
     LogicalSwitchStatefulACL(ls, _).
@@ -125,7 +125,7 @@  LogicalSwitchHasStatefulACL(ls, false) :-
     nb::Logical_Switch(._uuid = ls),
     not LogicalSwitchStatefulACL(ls, _).
 
-relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
+output relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
 
 LogicalSwitchHasACLs(ls, true) :-
     LogicalSwitchACL(ls, _).
@@ -170,7 +170,7 @@  LogicalSwitchWithDNSRecords(ls) :-
     nb::DNS(._uuid = dns_uuid, .records = records),
     not records.is_empty().
 
-relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records: bool)
+output relation LogicalSwitchHasDNSRecords(ls: uuid, has_dns_records: bool)
 
 LogicalSwitchHasDNSRecords(ls, true) :-
     LogicalSwitchWithDNSRecords(ls).
@@ -186,7 +186,7 @@  LogicalSwitchHasNonRouterPort0(ls_uuid) :-
     lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
     lsp.__type != "router".
 
-relation LogicalSwitchHasNonRouterPort(ls: uuid, has_non_router_port: bool)
+output relation LogicalSwitchHasNonRouterPort(ls: uuid, has_non_router_port: bool)
 LogicalSwitchHasNonRouterPort(ls, true) :-
     LogicalSwitchHasNonRouterPort0(ls).
 LogicalSwitchHasNonRouterPort(ls, false) :-
@@ -285,7 +285,7 @@  SwitchLBVIP(sw_uuid, lb, vip, backends) :-
     var kv = FlatMap(vips),
     (var vip, var backends) = kv.
 
-relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
+output relation LogicalSwitchHasLBVIP(sw_uuid: uuid, has_lb_vip: bool)
 LogicalSwitchHasLBVIP(sw_uuid, true) :-
     SwitchLBVIP(.sw_uuid = sw_uuid).
 LogicalSwitchHasLBVIP(sw_uuid, false) :-