On Mon, Aug 19, 2024 at 7:11 PM Han Zhou <[email protected]> wrote:
>
> On Mon, Aug 19, 2024 at 9:18 AM <[email protected]> wrote:
> >
> > From: Numan Siddique <[email protected]>
> >
> > Commit 8d13579bf5b3 creates a chassisredirect port for a logical switch
> > port (of type 'router') in certain scenarios and 'op->nbsp' can be NULL.
> > The following crash is reported by Sanitizer.
> >
> > ==84927==ERROR: AddressSanitizer: SEGV on unknown address
> > ==84927==The signal is caused by a READ memory access.
> > ==84927==Hint: address points to the zero page.
> >     #0 0x57ab3854f04a in hmap_first_with_hash 
> > ovn/ovs/./include/openvswitch/hmap.h:360:38
> >     #1 0x57ab3854fedf in smap_find__ ovn/ovs/lib/smap.c:421:5
> >     #2 0x57ab3854f7b8 in smap_get_node ovn/ovs/lib/smap.c:217:12
> >     #3 0x57ab3854f74f in smap_get_def ovn/ovs/lib/smap.c:208:30
> >     #4 0x57ab3854f726 in smap_get ovn/ovs/lib/smap.c:200:12
> >     #5 0x57ab3854f862 in smap_get_int ovn/ovs/lib/smap.c:240:25
> >     #6 0x57ab383222eb in ovn_port_assign_requested_tnl_id 
> > ovn/northd/northd.c:4372:27
> >     #7 0x57ab383072fa in build_ports ovn/northd/northd.c:4454:9
> >     #8 0x57ab38301457 in ovnnb_db_run ovn/northd/northd.c:18023:5
> >     #9 0x57ab3841d99e in en_northd_run ovn/northd/en-northd.c:137:5
> >     #10 0x57ab384599b2 in engine_recompute ovn/lib/inc-proc-eng.c:411:5
> >     #11 0x57ab38459d6e in engine_run_node ovn/lib/inc-proc-eng.c:473:9
> >     #12 0x57ab38459ec3 in engine_run ovn/lib/inc-proc-eng.c:524:9
> >     #13 0x57ab38430c5d in inc_proc_northd_run 
> > ovn/northd/inc-proc-northd.c:420:5
> >     #14 0x57ab3841bb2f in main ovn/northd/ovn-northd.c:970:32
> >
> > This patch fixes this issue.  It also corrects some typo introduced by
> > the commit - changes the reference from "chassisresident" to
> > "chassisredirect".
> >
> > Fixes: 8d13579bf5b3 ("Add support for centralize routing for distributed gw 
> > ports.")
> > Reported-by: Felix Huettner <[email protected]>
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-dev/2024-August/416264.html
> > CC: Han Zhou <[email protected]>
> > Signed-off-by: Numan Siddique <[email protected]>
> > ---
> >  northd/northd.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 7ceed63ddb..73367b9104 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1126,7 +1126,7 @@ is_l3dgw_port(const struct ovn_port *op)
> >  /* This function returns true if 'op' is a chassis resident
> >   * derived port. False otherwise.
> >   * There are 2 ways to check if 'op' is chassis resident port.
> > - *  1. op->sb->type is "chassisresident"
> > + *  1. op->sb->type is "chassisredirect"
> >   *  2. op->primary_port is not NULL.  If op->primary_port is set,
> >   *     it means 'op' is derived from the ovn_port op->primary_port.
> >   *
> > @@ -2136,7 +2136,7 @@ create_cr_port(struct ovn_port *op, struct hmap 
> > *ports,
> >
> >      struct ovn_port *crp = ovn_port_find(ports, redirect_name);
> >      if (crp && crp->sb && crp->sb->datapath == op->od->sb) {
> > -        ovn_port_set_nb(crp, NULL, op->nbrp);
> > +        ovn_port_set_nb(crp, op->nbsp, op->nbrp);
> >          ovs_list_remove(&crp->list);
> >          ovs_list_push_back(both_dbs, &crp->list);
> >      } else {
> > @@ -2466,7 +2466,7 @@ join_logical_ports(const struct 
> > sbrec_port_binding_table *sbrec_pb_table,
> >      }
> >
> >
> > -    /* Create chassisresident port for the distributed gateway port's (DGP)
> > +    /* Create chassisredirect port for the distributed gateway port's (DGP)
> >       * peer if
> >       *  - DGP's router has only one DGP and
> >       *  - Its peer is a logical switch port and
> > @@ -12622,7 +12622,7 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port 
> > *op,
> >
> >      if (op->peer && op->peer->cr_port) {
> >          /* We don't add the below flows if the router port's peer has
> > -         * a chassisresident port.  That's because routing is centralized 
> > on
> > +         * a chassisredirect port.  That's because routing is centralized 
> > on
> >           * the gateway chassis for the router port networks/subnets.
> >           */
> >          return;
> > @@ -16349,7 +16349,7 @@ lrouter_check_nat_entry(const struct ovn_datapath 
> > *od,
> >      *distributed = false;
> >
> >      /* NAT cannnot be distributed if the DGP's peer
> > -     * has a chassisresident port (as the routing is centralized
> > +     * has a chassisredirect port (as the routing is centralized
> >       * on the gateway chassis for the DGP's networks/subnets.)
> >       */
> >      struct ovn_port *l3dgw_port = *nat_l3dgw_port;
> > --
> > 2.45.2
> >
>
> Thanks Numan.
> Acked-by: Han Zhou <[email protected]>

Thanks.  Applied to main and branch-24.09.

Numan


> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to