Re: [PATCH net v2 1/2] net: openvswitch: do not record user features when the change fails

[Minxi Hou <houminxi@gmail.com>]

On 2026-11-09 08:23, Sashiko wrote:
> [Low] Is this list of reference holders accurate? Only cls_flower and
> openvswitch call tc_skb_ext_tc_enable() and tc_skb_ext_tc_disable().

Correct, and v3 says it that way: cls_flower and OVS take and drop
references, and the bridge, the flow dissector and the OVS flow path only
read the key and depend on it staying on while they do.

> [High] Can every failed OVS_DP_CMD_NEW leak one tc_skb_ext_tc reference?
> [High] ovs_dp_reset_user_features() has the same gap.
> Would it make sense to add a tc_skb_ext_tc_disable() to the
> ovs_dp_cmd_new() error path and to ovs_dp_reset_user_features()?

Both are real, and v3 fixes them along with the one the series started
with. All the paths that change or clear the bit now go through
ovs_dp_set_user_features(), which takes or drops the reference as it
records the field, so the four places that write it can no longer
disagree with the count. The failing NEW is the datapath that never
reaches __dp_destroy(), and the reset runs ahead of it for the name that
already exists.

The two tests cover the reset path: an outdated client sends NEW for a
datapath that exists, and the test checks that the datapath keeps
answering sets afterwards.

pw-bot: cr