diff mbox series

[ovs-dev,ovn,v2] ovn-controller: Check for NULL before accessing ovsrec_open_vswitch row.

Message ID 20200220144340.2787935-1-numans@ovn.org
State Accepted
Headers show
Series [ovs-dev,ovn,v2] ovn-controller: Check for NULL before accessing ovsrec_open_vswitch row. | expand

Commit Message

Numan Siddique Feb. 20, 2020, 2:43 p.m. UTC
From: Numan Siddique <numans@ovn.org>

There are occasional crashes seen with OpenShift CI

****
gdb) bt
0  hmap_first_with_hash (hmap=0x128, hmap=0x128, hash=2563384147) at include/openvswitch/hmap.h:328
1  smap_find__ (smap=0x128, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", key_len=27, hash=2563384147) at lib/smap.c:402
2  0x0000558de19f2596 in smap_get_node (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval") at lib/smap.c:217
3  0x0000558de19f26cc in smap_get_def (def=0x0, key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:208
4  smap_get (key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:200
5  smap_get_int (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", def=def@entry=5) at lib/smap.c:240
6  0x0000558de192f1ac in get_ofctrl_probe_interval (ovs_idl=<optimized out>) at controller/ovn-controller.c:439
7  main (argc=12, argv=0x7ffcfaef1e28) at controller/ovn-controller.c:1917

*****

This patch fixes it.

Reported-by: Alexander Constantinescu <aconstan@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
v1 -> v2
-------
  * Fixed the incomplete commit message

 controller/ovn-controller.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Dumitru Ceara Feb. 20, 2020, 2:54 p.m. UTC | #1
On 2/20/20 3:43 PM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> There are occasional crashes seen with OpenShift CI
> 
> ****
> gdb) bt
> 0  hmap_first_with_hash (hmap=0x128, hmap=0x128, hash=2563384147) at include/openvswitch/hmap.h:328
> 1  smap_find__ (smap=0x128, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", key_len=27, hash=2563384147) at lib/smap.c:402
> 2  0x0000558de19f2596 in smap_get_node (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval") at lib/smap.c:217
> 3  0x0000558de19f26cc in smap_get_def (def=0x0, key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:208
> 4  smap_get (key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:200
> 5  smap_get_int (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", def=def@entry=5) at lib/smap.c:240
> 6  0x0000558de192f1ac in get_ofctrl_probe_interval (ovs_idl=<optimized out>) at controller/ovn-controller.c:439
> 7  main (argc=12, argv=0x7ffcfaef1e28) at controller/ovn-controller.c:1917
> 
> *****
> 
> This patch fixes it.
> 
> Reported-by: Alexander Constantinescu <aconstan@redhat.com>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
> v1 -> v2
> -------
>   * Fixed the incomplete commit message
> 
>  controller/ovn-controller.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 4d245ca28..386c9f006 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -436,9 +436,10 @@ static int
>  get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
>  {
>      const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
> -    return smap_get_int(&cfg->external_ids,
> -                        "ovn-openflow-probe-interval",
> -                        OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
> +    return !cfg ? OFCTRL_DEFAULT_PROBE_INTERVAL_SEC :
> +                  smap_get_int(&cfg->external_ids,
> +                               "ovn-openflow-probe-interval",
> +                               OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
>  }
>  
>  /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
> 

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru
Numan Siddique Feb. 20, 2020, 3:30 p.m. UTC | #2
On Thu, Feb 20, 2020 at 8:25 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 2/20/20 3:43 PM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > There are occasional crashes seen with OpenShift CI
> >
> > ****
> > gdb) bt
> > 0  hmap_first_with_hash (hmap=0x128, hmap=0x128, hash=2563384147) at include/openvswitch/hmap.h:328
> > 1  smap_find__ (smap=0x128, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", key_len=27, hash=2563384147) at lib/smap.c:402
> > 2  0x0000558de19f2596 in smap_get_node (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval") at lib/smap.c:217
> > 3  0x0000558de19f26cc in smap_get_def (def=0x0, key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:208
> > 4  smap_get (key=0x558de1ab278f "ovn-openflow-probe-interval", smap=<optimized out>) at lib/smap.c:200
> > 5  smap_get_int (smap=<optimized out>, key=key@entry=0x558de1ab278f "ovn-openflow-probe-interval", def=def@entry=5) at lib/smap.c:240
> > 6  0x0000558de192f1ac in get_ofctrl_probe_interval (ovs_idl=<optimized out>) at controller/ovn-controller.c:439
> > 7  main (argc=12, argv=0x7ffcfaef1e28) at controller/ovn-controller.c:1917
> >
> > *****
> >
> > This patch fixes it.
> >
> > Reported-by: Alexander Constantinescu <aconstan@redhat.com>
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> > v1 -> v2
> > -------
> >   * Fixed the incomplete commit message
> >
> >  controller/ovn-controller.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 4d245ca28..386c9f006 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -436,9 +436,10 @@ static int
> >  get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
> >  {
> >      const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
> > -    return smap_get_int(&cfg->external_ids,
> > -                        "ovn-openflow-probe-interval",
> > -                        OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
> > +    return !cfg ? OFCTRL_DEFAULT_PROBE_INTERVAL_SEC :
> > +                  smap_get_int(&cfg->external_ids,
> > +                               "ovn-openflow-probe-interval",
> > +                               OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
> >  }
> >
> >  /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
> >
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks Dumitru. I applied this patch to master and branch-20.03

Numan

>
> Thanks,
> Dumitru
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4d245ca28..386c9f006 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -436,9 +436,10 @@  static int
 get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
 {
     const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
-    return smap_get_int(&cfg->external_ids,
-                        "ovn-openflow-probe-interval",
-                        OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
+    return !cfg ? OFCTRL_DEFAULT_PROBE_INTERVAL_SEC :
+                  smap_get_int(&cfg->external_ids,
+                               "ovn-openflow-probe-interval",
+                               OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
 }
 
 /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and