On 7/27/26 6:52 PM, Ren Wei wrote:
> From: Zhiling Zou <[email protected]>
> 
> ovs_flow_cmd_new() looks for a duplicate flow before deciding whether the
> request should insert a new flow or update an existing one.  When the new
> request carries a UFID, the function first looks up the UFID, but falls
> back to a key lookup if the UFID is not found.
> 
> That fallback can find a flow which is identified by a different UFID, or

UFIDs are optional and it's the user's responsibility to make sure that flows
with the same key have the same UFID, as the key is the primary identifier.
It is valid to update the flow with the matching key regardless of the UFID
provided in the request.

> by its key.  The update path then replaces that flow's actions even
> though the request identified a different flow.  If the caller asked for
> an echoed notification, the reply skb was already sized from the request
> identifier, but ovs_flow_cmd_fill_info() serializes the matched flow's
> identifier.  A short request UFID can therefore make the fill fail with
> -EMSGSIZE and hit the BUG_ON() in the update path.
> 
> Track whether the matched flow was found by the same identifier as the
> request, and reject updates where the identifier does not match.  This
> keeps OVS_FLOW_CMD_NEW from retargeting a different flow and avoids
> building a reply for an identifier that the skb was not sized for.

This breaks a scenario where the userspace components are upgraded from
the version that doesn't support UFIDs (or has them manually disabled) to
a different version that has them supported (or enabled back).  After this
patch it would not be possible to keep working with the installed flows
without removing and re-installing them back.

A better solution that doesn't change the logic would be to just free and
re-allocate the reply in case of the identifier mismatch.  If the
reallocation will only happen in a rare mismatch case, it should be fine to
do it under the mutex, while keeping the allocation outside of the mutex
for the common path.

Note: reallocation should happen before updating actions, as we must not
fail the request or fail to reply once the actions are updated.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to