On 7 Jan 2026, at 20:29, Aaron Conole wrote:
> Eelco Chaudron via dev <[email protected]> writes:
>
>> This commit introduces the initial framework and APIs to support
>> the hardware offload dpif provider.
>>
>> Signed-off-by: Eelco Chaudron <[email protected]>
>>
First of all, thanks for trying to work your way around this monster patch
series ;)
//Eelco
[...]
>>
>> +
>> +static int
>> +dpif_offload_attach_dp_offload(struct dpif *dpif,
>> + struct dp_offload *dp_offload)
>> + OVS_REQUIRES(dpif_offload_mutex)
>> +{
>> + ovsrcu_set(&dpif->dp_offload, dp_offload);
>> + ovs_refcount_ref(&dp_offload->ref_cnt);
>
> Would it make sense to use the return of ovs_refcount_try_ref_rcu here?
> I'm thinking in case the offload is somehow reattached. Otherwise, this
> return value seems not useful here. Then you'd want to invert the test
> like: try = success -> rcu_set()
>
> It's really a minor nit, though. I don't think it should actually cause
> any issues, but it seems strange to introduce an interface that doesn't
> really make sense.
I guess that adding a comment in this function makes sense, as there is
already one in the ovs_refcount_unref() path.
I also agree that the return value in this function absolutely did not make
sense, it was a leftover from earlier rework.
Below is the updated function for v5.
static void
dpif_offload_attach_dp_offload(struct dpif *dpif,
struct dp_offload *dp_offload)
OVS_REQUIRES(dpif_offload_mutex)
{
/* When called, 'dp_offload' should still have a refcount > 0, which is
* guaranteed by holding the lock from the shash lookup up to this point.
* If, for any reason, the refcount is not > 0, ovs_refcount_ref() will
* assert. */
ovs_refcount_ref(&dp_offload->ref_cnt);
ovsrcu_set(&dpif->dp_offload, dp_offload);
}
>> + return 0;
>> +}
[...]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
