Re: [PATCH net-next 1/2] netlink: specs: add ovs_flow del and set operations

[Minxi Hou <houminxi@gmail.com>]

On Thu, 8 Oct 2026, netdev-bot+sashiko@kernel.org wrote:
> [Medium] Should probe be declared as type: flag in flow-attrs?

Yes. The kernel policy marks both probe and clear as NLA_FLAG, and clear
was already a flag in the spec. probe stayed binary only because no
request listed it, so nothing exercised the mismatch. v2 makes it a flag.

> [Low] Should probe be listed on del as well? ovs_flow_cmd_del() reads it.

Yes, and v2 adds it. ovs_flow_cmd_del() reads OVS_FLOW_ATTR_PROBE to
decide whether to log the error. set listed it and del did not.

> [Low] Is the set doc complete? Replacing actions needs a key even
> when a ufid is present.

The doc was wrong. ovs_nla_init_match_and_action() returns -EINVAL for
actions with no key, whether or not a ufid is there. A ufid alone is
enough for a stats or clear update. v2 says so.

> [Low] Neither del nor set describes a reply or a notification.

Both handlers build a message and send it with ovs_notify(), and
ovs_must_notify() returns true when the request sets NLM_F_ECHO or
the multicast group has a listener. A client that wants the message
subscribes to the group.

A do() call does not get those attributes back. I checked on a live
kernel: new, set and del all return an ack, and only get returns the
flow attributes. The existing new op and the datapath and vport specs
describe the same thing the same way, which the review also notes.

Listing a reply would tell a generated client that do() returns
attributes it never receives. I left it off.