Hi Numan,

On 3/21/22 20:00, Numan Siddique wrote:
On Mon, Mar 14, 2022 at 7:10 AM Mohammad Heib <[email protected]> wrote:
currently pinctrl main thread uses some shash lists
that were supplied by ovn-controller main thread to prepare
and send IPv6 RAs, those lists are not updated properly when
LRP is deleted and can cause some invalid memory access
in the pinctrl module.

This patch handles such changes and update those lists
to avoid invalid memory access.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2052945
Signed-off-by: Mohammad Heib <[email protected]>

---
  controller/binding.c | 17 +++++++++++------
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 4d62b0858..ebbaae9ac 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -489,12 +489,14 @@ update_active_pb_ras_pd(const struct sbrec_port_binding 
*pb,
      bool ras_pd_conf = smap_get_bool(&pb->options, conf, false);
      struct shash_node *iter = shash_find(map, pb->logical_port);
      struct pb_ld_binding *ras_pd = iter ? iter->data : NULL;
+    bool deleted = sbrec_port_binding_is_deleted(pb);

-    if (iter && !ras_pd_conf) {
+    if (iter && (!ras_pd_conf || deleted)) {
          shash_delete(map, iter);
          free(ras_pd);
          return;
      }
+
      if (ras_pd_conf) {
          if (!ras_pd) {
              ras_pd = xzalloc(sizeof *ras_pd);
@@ -2338,11 +2340,9 @@ delete_done:

      SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
                                                 b_ctx_in->port_binding_table) {
-        /* Loop to handle create and update changes only. */
-        if (sbrec_port_binding_is_deleted(pb)) {
-            continue;
-        }
-
+        /* handle port changes/deletion and update the local active ports ipv6
+         * releated lists.
+         */
Thanks Mohammad for fixing this issue.

The loop in which you've modified the code handles the port binding
create/updates.
I think its a bit odd to handle port binding deletions here.
And we have a loop just above to handle the port binding deletions.

I'd suggest to delete the port binding related entry from the shash's
- b_ctx_out->local_active_ports_ipv6_pd
and b_ctx_out->local_active_ports_ras in the first
'SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED()' loop.

You could probably add a new function delete_active_pb_ras_pd() (or
with a  better name) to delete
the entry from these shash.
thank you for your review:)
i submitted v2 with your suggested changes.
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
thanks,




          update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths,
                                  b_ctx_out->local_active_ports_ipv6_pd,
                                  "ipv6_prefix_delegation");
@@ -2351,6 +2351,11 @@ delete_done:
                                  b_ctx_out->local_active_ports_ras,
                                  "ipv6_ra_send_periodic");

+        /* Loop to handle create and update changes only. */
+        if (sbrec_port_binding_is_deleted(pb)) {
+            continue;
+        }
+
          enum en_lport_type lport_type = get_lport_type(pb);

          struct binding_lport *b_lport =
--
2.34.1

_______________________________________________
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