Re: [ovs-dev] [PATCH ovn] controller: Add IDL memory reports.
On Tue, Nov 30, 2021 at 4:50 AM Dumitru Ceara wrote:
>
> On 11/29/21 23:21, Numan Siddique wrote:
> > On Tue, Nov 23, 2021 at 10:02 AM Dumitru Ceara wrote:
> >>
> >> OVS commit 066741d9c5ca ("ovsdb-idl: Add memory report function.")
> >> enhanced the IDL to track memory usage. Use it in ovn-controller for
> >> the Southbound DB and local OVS DB IDLs.
> >>
> >> Signed-off-by: Dumitru Ceara
> >> ---
> >> controller/ovn-controller.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> >> index 29c1a05b2..fa1ff13bd 100644
> >> --- a/controller/ovn-controller.c
> >> +++ b/controller/ovn-controller.c
> >> @@ -3444,6 +3444,8 @@ main(int argc, char *argv[])
> >> ofctrl_get_memory_usage();
> >> if_status_mgr_get_memory_usage(if_mgr, );
> >> local_datapath_memory_usage();
> >> +ovsdb_idl_get_memory_usage(ovnsb_idl_loop.idl, );
> >> +ovsdb_idl_get_memory_usage(ovs_idl_loop.idl, );
> >
> > Thanks for the patch. I've one question.
>
> Thanks for the review!
>
> >
> > The function ovsdb_idl_get_memory_usage() adds 2 items to the "usage"
> > simap - "idl-cells" and "idl-outstanding-txns" (if its value is
> > greater than zero).
> > After the ovsdb_idl_get_memory_usage() for OVS IDL is called, the
> > "idl-cells" value in usage will be the sum of SB IDL cells and OVS IDL
> > cells.
>
> Right.
>
> > That's intentional right ? I think it makes sense. Just wanted to
> > check with you.
>
> Yes, it's due to how ovsdb_idl_get_memory_usage() currently works.
>
> > Is there a value in reporting as - "sb-idl-cells=", ovs-idl-cells=.."
> > ?
>
> Good point, I think there is. And I think we can do that by changing
> ovsdb_idl_get_memory_usage() to prefix the "usage" simap item keys with
> the DB name.
>
> I sent a patch for that in OVS (the OVN bits don't need to change):
> https://patchwork.ozlabs.org/project/openvswitch/patch/20211130094757.23210-1-dce...@redhat.com/
Thanks. Since there would be no change in OVN. I applied this
patch. Probably we have to bump the ovs submodule
once your OVS patch is merged.
Numan
>
> Thanks,
> Dumitru
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] controller: Add IDL memory reports.
On 11/29/21 23:21, Numan Siddique wrote:
> On Tue, Nov 23, 2021 at 10:02 AM Dumitru Ceara wrote:
>>
>> OVS commit 066741d9c5ca ("ovsdb-idl: Add memory report function.")
>> enhanced the IDL to track memory usage. Use it in ovn-controller for
>> the Southbound DB and local OVS DB IDLs.
>>
>> Signed-off-by: Dumitru Ceara
>> ---
>> controller/ovn-controller.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index 29c1a05b2..fa1ff13bd 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -3444,6 +3444,8 @@ main(int argc, char *argv[])
>> ofctrl_get_memory_usage();
>> if_status_mgr_get_memory_usage(if_mgr, );
>> local_datapath_memory_usage();
>> +ovsdb_idl_get_memory_usage(ovnsb_idl_loop.idl, );
>> +ovsdb_idl_get_memory_usage(ovs_idl_loop.idl, );
>
> Thanks for the patch. I've one question.
Thanks for the review!
>
> The function ovsdb_idl_get_memory_usage() adds 2 items to the "usage"
> simap - "idl-cells" and "idl-outstanding-txns" (if its value is
> greater than zero).
> After the ovsdb_idl_get_memory_usage() for OVS IDL is called, the
> "idl-cells" value in usage will be the sum of SB IDL cells and OVS IDL
> cells.
Right.
> That's intentional right ? I think it makes sense. Just wanted to
> check with you.
Yes, it's due to how ovsdb_idl_get_memory_usage() currently works.
> Is there a value in reporting as - "sb-idl-cells=", ovs-idl-cells=.." ?
Good point, I think there is. And I think we can do that by changing
ovsdb_idl_get_memory_usage() to prefix the "usage" simap item keys with
the DB name.
I sent a patch for that in OVS (the OVN bits don't need to change):
https://patchwork.ozlabs.org/project/openvswitch/patch/20211130094757.23210-1-dce...@redhat.com/
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] controller: Add IDL memory reports.
On Tue, Nov 23, 2021 at 10:02 AM Dumitru Ceara wrote:
>
> OVS commit 066741d9c5ca ("ovsdb-idl: Add memory report function.")
> enhanced the IDL to track memory usage. Use it in ovn-controller for
> the Southbound DB and local OVS DB IDLs.
>
> Signed-off-by: Dumitru Ceara
> ---
> controller/ovn-controller.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 29c1a05b2..fa1ff13bd 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -3444,6 +3444,8 @@ main(int argc, char *argv[])
> ofctrl_get_memory_usage();
> if_status_mgr_get_memory_usage(if_mgr, );
> local_datapath_memory_usage();
> +ovsdb_idl_get_memory_usage(ovnsb_idl_loop.idl, );
> +ovsdb_idl_get_memory_usage(ovs_idl_loop.idl, );
Thanks for the patch. I've one question.
The function ovsdb_idl_get_memory_usage() adds 2 items to the "usage"
simap - "idl-cells" and "idl-outstanding-txns" (if its value is
greater than zero).
After the ovsdb_idl_get_memory_usage() for OVS IDL is called, the
"idl-cells" value in usage will be the sum of SB IDL cells and OVS IDL
cells.
That's intentional right ? I think it makes sense. Just wanted to
check with you.
Is there a value in reporting as - "sb-idl-cells=", ovs-idl-cells=.." ?
Thanks
Numan
> memory_report();
> simap_destroy();
> }
> --
> 2.27.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] controller: Add IDL memory reports.
OVS commit 066741d9c5ca ("ovsdb-idl: Add memory report function.")
enhanced the IDL to track memory usage. Use it in ovn-controller for
the Southbound DB and local OVS DB IDLs.
Signed-off-by: Dumitru Ceara
---
controller/ovn-controller.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 29c1a05b2..fa1ff13bd 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3444,6 +3444,8 @@ main(int argc, char *argv[])
ofctrl_get_memory_usage();
if_status_mgr_get_memory_usage(if_mgr, );
local_datapath_memory_usage();
+ovsdb_idl_get_memory_usage(ovnsb_idl_loop.idl, );
+ovsdb_idl_get_memory_usage(ovs_idl_loop.idl, );
memory_report();
simap_destroy();
}
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
