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.
> 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
