[PATCH net-next v8 3/3] netlink: specs: correct the ovs specs where they do not match the kernel

[Minxi Hou <houminxi@gmail.com>]

The vport netnsid attribute is signed in the kernel: vport_policy declares
it NLA_S32 and ovs_vport_cmd_fill_info() sends it with nla_put_s32(). A
client reading it as unsigned turns the -1 that peernet2id_alloc() returns
when it cannot assign an identifier into 4294967295. Declare it s32, and
say the same in the uapi comment, which also never said the value is -1
when no identifier could be assigned.

Two dumps shared a request list with their get operation, while the dump
handlers read no request attribute. Each dump now has a request list of its
own with no attributes, so neither advertises an attribute the kernel would
ignore.

Four lists named attributes their handler never reads or sends. The vport
delete goes through lookup_vport() and reads no type. The datapath reply
does not carry upcall-pid, which the uapi reserves for the new request, so
the reply list drops it and the ynl selftest stops printing that field. The
vport new request named the nested options, which the kernel accepts and
ignores. The packet miss event shared its list with the userspace action
event, which alone fills userdata, actions, the tunnel key and the length
of a truncated packet, so the miss event now lists only what a miss sends.

The vport get request can also be answered by port number, because
lookup_vport() falls back to it when the request carries no name, so list
port-no there. The datapath get is served by the name, or by the header
ifindex when the name is absent, which its doc now says, and all four ovs
specs document the header struct the same way. The vport new doc gains the
constraint that an ifindex is only accepted for an internal vport.

The uapi note on the vport name now covers requests other than new and
dump, and names the header member the lookup uses. The note on options says
what the kernel does with the attribute instead of leaving it to the vport
type.

The packet spec declared neither the validate flags nor the admin
permission that the kernel registers on the execute operation.

The vport spec also declared family version 2 where the kernel registers 1.

None of this comes from the set operations added earlier in the series; the
specs have been wrong since the families were introduced.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
 Documentation/netlink/specs/ovs_datapath.yaml | 16 +++++++++----
 Documentation/netlink/specs/ovs_packet.yaml   | 20 +++++++++++-----
 Documentation/netlink/specs/ovs_vport.yaml    | 24 ++++++++++++-------
 include/uapi/linux/openvswitch.h              | 14 ++++++-----
 tools/net/ynl/tests/ovs.c                     |  5 ++--
 5 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/Documentation/netlink/specs/ovs_datapath.yaml b/Documentation/netlink/specs/ovs_datapath.yaml
index 6232760224dc..94ab15251e2a 100644
--- a/Documentation/netlink/specs/ovs_datapath.yaml
+++ b/Documentation/netlink/specs/ovs_datapath.yaml
@@ -12,6 +12,8 @@ definitions:
   -
     name: ovs-header
     type: struct
+    doc: |
+      Header for OVS Generic Netlink messages.
     members:
       -
         name: dp-ifindex
@@ -118,24 +120,28 @@ operations:
   list:
     -
       name: get
-      doc: Get / dump OVS data path configuration and state
+      doc: |
+        Get one OVS data path by name, or by the ifindex in the header when
+        the request carries no name.
       value: 3
       attribute-set: datapath
       dont-validate: [strict, dump]
-      do: &dp-get-op
+      do:
         request:
           attributes:
             - name
-        reply:
+        reply: &dp-reply
           attributes:
             - name
-            - upcall-pid
             - stats
             - megaflow-stats
             - user-features
             - masks-cache-size
             - per-cpu-pids
-      dump: *dp-get-op
+      dump:
+        request:
+          attributes: []
+        reply: *dp-reply
     -
       name: new
       doc: |
diff --git a/Documentation/netlink/specs/ovs_packet.yaml b/Documentation/netlink/specs/ovs_packet.yaml
index e4c5f24b3b01..a9ee84be6e22 100644
--- a/Documentation/netlink/specs/ovs_packet.yaml
+++ b/Documentation/netlink/specs/ovs_packet.yaml
@@ -22,6 +22,8 @@ definitions:
   -
     name: ovs-header
     type: struct
+    doc: |
+      Header for OVS Generic Netlink messages.
     members:
       -
         name: dp-ifindex
@@ -97,14 +99,10 @@ operations:
       value: 1
       attribute-set: packet
       event:
-        attributes: &event-attrs
+        attributes:
           - packet
           - key
-          - userdata
-          - actions
-          - egress-tun-key
           - mru
-          - len
           - hash
     -
       name: action
@@ -112,12 +110,22 @@ operations:
       value: 2
       attribute-set: packet
       event:
-        attributes: *event-attrs
+        attributes: &event-attrs
+          - packet
+          - key
+          - userdata
+          - actions
+          - egress-tun-key
+          - mru
+          - len
+          - hash
     -
       name: execute
       doc: Apply actions to a packet.
       value: 3
       attribute-set: packet
+      dont-validate: [strict, dump]
+      flags: [uns-admin-perm]
       do:
         request:
           attributes:
diff --git a/Documentation/netlink/specs/ovs_vport.yaml b/Documentation/netlink/specs/ovs_vport.yaml
index af7ee56f5f90..7626b578c568 100644
--- a/Documentation/netlink/specs/ovs_vport.yaml
+++ b/Documentation/netlink/specs/ovs_vport.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
 ---
 name: ovs_vport
-version: 2
+version: 1
 protocol: genetlink-legacy
 uapi-header: linux/openvswitch.h
 
@@ -12,6 +12,8 @@ definitions:
   -
     name: ovs-header
     type: struct
+    doc: |
+      Header for OVS Generic Netlink messages.
     members:
       -
         name: dp-ifindex
@@ -116,7 +118,7 @@ attribute-sets:
           min: 0
       -
         name: netnsid
-        type: u32
+        type: s32
       -
         name: upcall-stats
         type: nest
@@ -128,7 +130,9 @@ operations:
   list:
     -
       name: new
-      doc: Create a new OVS vport
+      doc: |
+        Create a new OVS vport. An ifindex is accepted only for an
+        internal vport; any other type returns -EOPNOTSUPP.
       attribute-set: vport
       dont-validate: [strict, dump]
       flags: [uns-admin-perm]
@@ -140,7 +144,6 @@ operations:
             - upcall-pid
             - port-no
             - ifindex
-            - options
     -
       name: del
       doc: Delete existing OVS vport from a data path
@@ -151,17 +154,19 @@ operations:
         request:
           attributes:
             - port-no
-            - type
             - name
     -
       name: get
-      doc: Get / dump OVS vport configuration and state
+      doc: |
+        Get one OVS vport by name, or by port number when the request
+        carries no name.
       attribute-set: vport
       dont-validate: [strict, dump]
-      do: &vport-get-op
+      do:
         request:
           attributes:
             - name
+            - port-no
         reply: &dev-all
           attributes:
             - port-no
@@ -172,7 +177,10 @@ operations:
             - ifindex
             - netnsid
             - upcall-stats
-      dump: *vport-get-op
+      dump:
+        request:
+          attributes: []
+        reply: *dev-all
     -
       name: set
       doc: |
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index bd77e6fb4a95..b111eb9a0066 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -293,7 +293,9 @@ enum ovs_vport_type {
  * packets sent or received through the vport.
  * @OVS_VPORT_ATTR_IFINDEX: Provides the ifindex of a vport, or sets the desired
  * ifindex while creating a new vport with type %OVS_VPORT_TYPE_INTERNAL.
- * @OVS_VPORT_ATTR_NETNSID: Provides the netns id of the vport if it's not local.
+ * @OVS_VPORT_ATTR_NETNSID: Provides the netns id of the vport if it's not
+ * local, as a signed 32-bit value.  The value is -1 when the identifier could
+ * not be assigned.
  * @OVS_VPORT_ATTR_UPCALL_STATS: Provides upcall statistics for a vport.
  * Contains nested %OVS_VPORT_UPCALL_ATTR_* attributes.
  *
@@ -303,12 +305,12 @@ enum ovs_vport_type {
  * For %OVS_VPORT_CMD_NEW requests, the %OVS_VPORT_ATTR_TYPE and
  * %OVS_VPORT_ATTR_NAME attributes are required.  %OVS_VPORT_ATTR_PORT_NO is
  * optional; if not specified a free port number is automatically selected.
- * Whether %OVS_VPORT_ATTR_OPTIONS is required or optional depends on the type
- * of vport.  None of currently supported vport types support options.
+ * None of currently supported vport types support options, so
+ * %OVS_VPORT_ATTR_OPTIONS is ignored in a %OVS_VPORT_CMD_NEW request and
+ * rejected with -EOPNOTSUPP in a %OVS_VPORT_CMD_SET request.
  *
- * For other requests, if %OVS_VPORT_ATTR_NAME is specified then it is used to
- * look up the vport to operate on; otherwise dp_idx from the &struct
- * ovs_header plus %OVS_VPORT_ATTR_PORT_NO determine the vport.
+ * For requests other than new and dump, if %OVS_VPORT_ATTR_NAME is specified
+ * then it is used to look up the vport to operate on; otherwise dp_ifindex from
  */
 enum ovs_vport_attr {
 	/* private: */
diff --git a/tools/net/ynl/tests/ovs.c b/tools/net/ynl/tests/ovs.c
index d49f5a8e647e..fea7609a68b9 100644
--- a/tools/net/ynl/tests/ovs.c
+++ b/tools/net/ynl/tests/ovs.c
@@ -16,9 +16,8 @@ static void ovs_print_datapath(struct __test_metadata *_metadata,
 		return;
 
 	EXPECT_TRUE((bool)dp->_hdr.dp_ifindex);
-	ksft_print_msg("%s(%d): pid:%u cache:%u\n",
-		       dp->name, dp->_hdr.dp_ifindex,
-		       dp->upcall_pid, dp->masks_cache_size);
+	ksft_print_msg("%s(%d): cache:%u\n",
+		       dp->name, dp->_hdr.dp_ifindex, dp->masks_cache_size);
 }
 
 FIXTURE(ovs)
-- 
2.56.0