On 15 Jan 2026, at 21:49, Eelco Chaudron wrote:
> On 15 Jan 2026, at 20:54, Aaron Conole wrote:
>
>> During code-review, it seems like it could be possible to publish a
>> flow into the cmap, and the hw offload thread assistance could miss
>> the appropriate flow. This situation probably didn't occur much in
>> practice, and the result might be just a dropped packet (although, I'm
>> not sure that it couldn't also result in a weird duplicate flow
>> getting installed due to the check in mark_to_flow_find that gets
>> called during dfc_processing).
>>
>> This change reorders the assignment to before the cmap_insert. The
>> cmap_insert should act as a barrier in this case to ensure that the
>> write to flow->mark is ready before the read (after the
>> CMAP_FOR_EACH_WITH_HASH).
>>
>> Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id")
>> Signed-off-by: Aaron Conole <[email protected]>
>
> Hi Aaron,
>
> I haven’t reviewed the code yet, but it should be noted that this does not
> apply to main after the offload series was merged. So this would be for older
> streams only, right?
Actually, the review is quite simple, and the change makes sense, the flow
structure should be fully initialized before insertion, nice catch ;)
So for backporting,
Acked-by: Eelco Chaudron <[email protected]>
> Cheers,
> Eelco
>
>> ---
>> lib/dpif-netdev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 224ce7086f..31b95f153c 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -2600,10 +2600,10 @@ mark_to_flow_associate(const uint32_t mark, struct
>> dp_netdev_flow *flow)
>> unsigned int tid = netdev_offload_thread_id();
>> dp_netdev_flow_ref(flow);
>>
>> + flow->mark = mark;
>> cmap_insert(&dp_offload_threads[tid].mark_to_flow,
>> CONST_CAST(struct cmap_node *, &flow->mark_node),
>> hash_int(mark, 0));
>> - flow->mark = mark;
>>
>> VLOG_DBG("Associated dp_netdev flow %p with mark %u mega_ufid "UUID_FMT,
>> flow, mark, UUID_ARGS((struct uuid *) &flow->mega_ufid));
>> --
>> 2.51.0
>>
>> _______________________________________________
>> 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
