Re: [PATCH net-next v5 0/2] netlink: specs: fill in the missing ovs operations

[Minxi Hou <houminxi@gmail.com>]

Sashiko reviewed v5 and raised four things. Answers below.

The ufid length one stands. get_ufid_len() returns 0 for a ufid shorter
than 1 byte or longer than 16, and ovs_nla_get_ufid() then reports it as
absent, so a del with no key and an invalid ufid does flush the data
path. I will say that in the del and key docs, and give the attribute the
range the kernel enforces. A new flow that carries an invalid ufid is
indexed by its unmasked key instead, so the ufid doc needs the same
qualification.

ifindex is u32 in the spec and signed in the kernel, and no datapath
request used the attribute before this series, so it went unnoticed. v6
declares it s32 with a minimum of 0.

dont-validate is missing. These ops carry GENL_DONT_VALIDATE_STRICT |
GENL_DONT_VALIDATE_DUMP, so new, del and set in both specs should say so,
the way fou and team do.

The last one is a real bug and it is older than this series.
ovs_dp_change() stores the new features in dp->user_features before
calling ovs_dp_set_upcall_portids(), and that call fails on an empty
per-cpu-pids. The request returns an error with the TC_RECIRC_SHARING bit
already recorded and tc_skb_ext_tc_enable() not run, so the next request
that clears the bit calls tc_skb_ext_tc_disable() for an enable that
never happened. The same key is shared with cls_flower. I will send a fix
separately rather than mix kernel code into a spec series.