Eelco Chaudron via dev <[email protected]> writes:
> 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]>
Thanks Eelco - I'll fix the 'author' and plan to apply down to 3.3 by
tomorrow EOB.
>> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
