On 2/10/26 2:23 PM, Dumitru Ceara wrote:
> Hi Ales,
>
> Thanks for the fix!
>
> On 2/10/26 1:57 PM, Ales Musil wrote:
>> The logical ports are always populated at the database level.
>> However, when conditional monitoring is enabled it can happen
>> after ovn-controller starts that we will get Advertised MAC
>> binding rows with LSPs that are not yet monitored. This results
>> in IDL setting the pointer as NULL. Which results in a crash
>> as we try to deref the NULL pointer:
>>
>> neighbor_get_relevant_port_binding.lto_priv.0 (ovn-controller + 0x4fc8f)
>
> Nit: indent left.
>
>> en_neighbor_run.lto_priv.0 (ovn-controller + 0x87512)
>> engine_recompute (ovn-controller + 0xc9e20)
>> engine_run (ovn-controller + 0xca175)
>> main (ovn-controller + 0x289aa)
>>
>> To prevent that skip the entries with NULL logical_port, we will
>> run the node again when the LSPs are actually monitored.
>>
>> Fixes: 499ddafbf160 ("controller: Advertise local EVPN type2 routes in
>> ovn-controller.")
>> Reported-at: https://issues.redhat.com/browse/FDP-3100
>> Signed-off-by: Ales Musil <[email protected]>
>> ---
>> controller/neighbor.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/controller/neighbor.c b/controller/neighbor.c
>> index e77b41d29..eae5eadb2 100644
>> --- a/controller/neighbor.c
>> +++ b/controller/neighbor.c
>> @@ -314,6 +314,13 @@ neighbor_collect_ip_mac_to_advertise(
>> const struct sbrec_advertised_mac_binding *adv_mb;
>> SBREC_ADVERTISED_MAC_BINDING_FOR_EACH_EQUAL (adv_mb, target,
>> n_ctx_in->sbrec_amb_by_dp)
>> {
>> + if (!adv_mb->logical_port) {
>> + /* Skip the NULL logical_port, this can happen in some cases
>> + * during controller startup. See
>> + * https://issues.redhat.com/browse/FDP-3114 for more details.
>> */
>> + continue;
>> + }
>> +
>> const struct sbrec_port_binding *pb =
>> neighbor_get_relevant_port_binding(n_ctx_in->sbrec_pb_by_name,
>> adv_mb->logical_port);
>
> Looks good to me, it's unfortunate but until FDP-3114 is fixed we have
> to live with this check. From what I can tell the other opt-in tables
> don't have this issue because the ports they reference will always be
> included in the monitor condition. Therefore:
>
> Acked-by: Dumitru Ceara <[email protected]>
>
Actually, I was applying a different patch so I also pushed this one to
main too.
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
