On Tue, May 17, 2022 at 6:04 PM Numan Siddique <[email protected]> wrote:
> > > On Tue, May 10, 2022 at 1:40 AM Ales Musil <[email protected]> wrote: > >> When the "ovn-chassis-mac-mappings" had wrong format it would >> crash the ovn-controller as it tried to deref NULL pointer. >> >> Add check if the token is a valid pointer, if not log warning >> and skip this value. >> >> Reported-at: https://bugzilla.redhat.com/2082341 >> Signed-off-by: Ales Musil <[email protected]> >> > > Thanks for fixing this. > > --- >> controller/physical.c | 6 ++++++ >> tests/ovn-controller.at | 28 ++++++++++++++++++++++++++++ >> 2 files changed, 34 insertions(+) >> >> diff --git a/controller/physical.c b/controller/physical.c >> index 734444320..71e22cbf0 100644 >> --- a/controller/physical.c >> +++ b/controller/physical.c >> @@ -426,6 +426,12 @@ populate_remote_chassis_macs(const struct >> sbrec_chassis *my_chassis, >> char *save_ptr2 = NULL; >> char *chassis_mac_bridge = strtok_r(token, ":", &save_ptr2); >> char *chassis_mac_str = strtok_r(NULL, "", &save_ptr2); >> + if(!chassis_mac_str) { >> > > As per the coding guidelines, this should be > if (!chassis_mac_str) > > I think Mark/Han can fix it before applying it. > > Acked-by: Numan Siddique <[email protected]> > > Numan > Fixed in v2. Thanks, Ales > > + VLOG_WARN("Parsing of ovn-chassis-mac-mappings failed >> for: " >> + "\"%s\", the correct format is >> \"br-name1:MAC1\".", >> + token); >> + continue; >> + } >> struct remote_chassis_mac *remote_chassis_mac = NULL; >> remote_chassis_mac = xmalloc(sizeof *remote_chassis_mac); >> hmap_insert(&remote_chassis_macs, >> &remote_chassis_mac->hmap_node, >> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at >> index 88ec2f269..dcadab1f3 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -2154,3 +2154,31 @@ AT_CHECK_UNQUOTED([echo $lflow_run_1], [0], >> [$lflow_run_2 >> >> OVN_CLEANUP([hv1]) >> AT_CLEANUP >> + >> + >> +AT_SETUP([ovn-controller - check ovn-chassis-mac-mappings]) >> + >> +ovn_start >> + >> +net_add n1 >> +sim_add hv1 >> +as hv1 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.1 >> + >> +pid=$(cat hv1/ovn-controller.pid) >> + >> +# Add chassis with some ovn-chassis-mac-mappings >> +AT_CHECK([ovn-sbctl chassis-add foo geneve 127.0.0.2]) >> +AT_CHECK([ovn-sbctl set chassis foo >> other_config:ovn-chassis-mac-mappings="invalid1,invalid2,br1:00:00:00:00:00:00"]) >> +AT_CHECK([ovn-nbctl --wait=hv sync]) >> + >> +# Check if ovn-controller is still alive >> +AT_CHECK([ps $pid], [0], [ignore]) >> +# Check if we got warnings for invalid >> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed" >> hv1/ovn-controller.log | grep -q invalid1]) >> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed" >> hv1/ovn-controller.log | grep -q invalid2]) >> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed" >> hv1/ovn-controller.log | grep -q br1], [1]) >> + >> +OVN_CLEANUP([hv1]) >> +AT_CLEANUP >> -- >> 2.35.1 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- Ales Musil Senior Software Engineer - RHV Network Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
