[PATCH net-next 0/2] netlink: specs: complete OVS family coverage

[PATCH net-next 0/2] netlink: specs: complete OVS family coverage

[Minxi Hou]

This series adds the missing OVS_PACKET_FAMILY spec and a small
pyynl enhancement to support unicast notification receiving.

Patch 1 adds ovs_packet.yaml with MISS/ACTION events and the
EXECUTE operation, completing the set of OVS genetlink specs.

Patch 2 adds ntf_listen() to YnlFamily so that unicast
notifications (like OVS upcalls sent via genlmsg_unicast) can
be received through the existing poll_ntf/check_ntf path.

These two patches lay the groundwork for migrating the kernel
selftest ovs-dpctl.py from pyroute2 to in-tree pyynl.

Minxi Hou (2):
  netlink: specs: add OVS packet family specification
  tools: ynl: add unicast notification receive support

 Documentation/netlink/specs/ovs_packet.yaml | 138 ++++++++++++++++++++
 tools/net/ynl/pyynl/lib/ynl.py              |   4 +
 2 files changed, 142 insertions(+)
 create mode 100644 Documentation/netlink/specs/ovs_packet.yaml

-- 
2.53.0

[PATCH 1/2] netlink: specs: add OVS packet family specification

[Minxi Hou]

Add YAML netlink spec for the OVS_PACKET_FAMILY (ovs_packet).
This completes the set of OVS genetlink family specs (ovs_datapath,
ovs_flow, ovs_vport already exist).

The spec defines three operations: MISS (event), ACTION (event),
and EXECUTE (do). MISS and ACTION are kernel-to-userspace upcalls
sent via genlmsg_unicast(); EXECUTE is the only registered genl
operation.

Key, actions, and egress-tun-key attributes are typed as binary
rather than nest because the nested attribute definitions belong
to the ovs_flow spec and cross-spec references are not supported
by the YNL framework.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
 Documentation/netlink/specs/ovs_packet.yaml | 138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/netlink/specs/ovs_packet.yaml

diff --git a/Documentation/netlink/specs/ovs_packet.yaml b/Documentation/netlink/specs/ovs_packet.yaml
new file mode 100644
index 000000000000..7ef14b68df4e
--- /dev/null
+++ b/Documentation/netlink/specs/ovs_packet.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: ovs_packet
+version: 1
+protocol: genetlink-legacy
+uapi-header: linux/openvswitch.h
+
+doc: |
+  OVS packet execution over generic netlink.
+
+  Only OVS_PACKET_CMD_EXECUTE is exposed as a genl operation.
+  OVS_PACKET_CMD_MISS and OVS_PACKET_CMD_ACTION are kernel-to-userspace
+  upcalls sent via genlmsg_unicast() to the vport's upcall_pid and have
+  no associated genl_ops or multicast group.
+
+  Several attributes in the attribute set (userdata, egress-tun-key, len)
+  exist for the upcall path and are not used by the EXECUTE operation.
+  For EXECUTE, packet, key, and actions are mandatory (kernel returns
+  -EINVAL without them).
+
+definitions:
+  -
+    name: ovs-header
+    type: struct
+    members:
+      -
+        name: dp-ifindex
+        type: u32
+
+attribute-sets:
+  -
+    name: packet
+    name-prefix: ovs-packet-attr-
+    enum-name: ovs-packet-attrs
+    attributes:
+      -
+        name: packet
+        type: binary
+        doc: Packet data, from the start of the Ethernet header.
+        checks:
+          min-len: 14
+      -
+        name: key
+        type: binary
+        doc: |
+          Nested OVS_KEY_ATTR_* attributes, extracted flow key.
+          Defined as binary because the key attribute-set belongs to the
+          ovs_flow family spec; cross-spec references are not supported.
+      -
+        name: actions
+        type: binary
+        doc: |
+          Nested OVS_ACTION_ATTR_* attributes.
+          Defined as binary for the same reason as key.
+      -
+        name: userdata
+        type: binary
+        doc: Opaque userspace cookie from OVS_USERSPACE_ATTR_USERDATA.
+      -
+        name: egress-tun-key
+        type: binary
+        doc: Nested OVS_TUNNEL_KEY_ATTR_* for output tunnel metadata.
+      -
+        name: unused1
+        type: unused
+      -
+        name: unused2
+        type: unused
+      -
+        name: probe
+        type: flag
+        doc: Packet operation is a feature probe, error logging suppressed.
+      -
+        name: mru
+        type: u16
+        doc: Maximum received IP fragment size.
+      -
+        name: len
+        type: u32
+        doc: Packet size before truncation.
+      -
+        name: hash
+        type: u64
+        doc: Packet hash, low 32 bits are skb hash, upper bits are flags.
+      -
+        name: upcall-pid
+        type: u32
+        doc: Netlink PID to use for upcalls during EXECUTE processing.
+
+operations:
+  fixed-header: ovs-header
+  name-prefix: ovs-packet-cmd-
+  list:
+    -
+      name: miss
+      doc: Notify userspace of a flow table miss for a received packet.
+      value: 1
+      attribute-set: packet
+      event:
+        attributes:
+          - packet
+          - key
+          - userdata
+          - actions
+          - egress-tun-key
+          - mru
+          - len
+          - hash
+    -
+      name: action
+      doc: Notify userspace as requested by an OVS_ACTION_ATTR_USERSPACE action.
+      value: 2
+      attribute-set: packet
+      event:
+        attributes:
+          - packet
+          - key
+          - userdata
+          - actions
+          - egress-tun-key
+          - mru
+          - len
+          - hash
+    -
+      name: execute
+      doc: Apply actions to a packet.
+      value: 3
+      attribute-set: packet
+      do:
+        request:
+          attributes:
+            - packet
+            - key
+            - actions
+            - probe
+            - mru
+            - hash
+            - upcall-pid
-- 
2.53.0

Re: [PATCH 1/2] netlink: specs: add OVS packet family specification

[Donald Hunter]

Minxi Hou <houminxi@gmail.com> writes:

> +operations:
> +  fixed-header: ovs-header
> +  name-prefix: ovs-packet-cmd-
> +  list:
> +    -
> +      name: miss
> +      doc: Notify userspace of a flow table miss for a received packet.
> +      value: 1
> +      attribute-set: packet
> +      event:
> +        attributes:

Instead of repeating the same attributes, this attribute list could be
declared with an anchor name such as &event-attrs which could be
referenced in the action event. E.g.

           attributes: &event-attrs

> +          - packet
> +          - key
> +          - userdata
> +          - actions
> +          - egress-tun-key
> +          - mru
> +          - len
> +          - hash
> +    -
> +      name: action
> +      doc: Notify userspace as requested by an OVS_ACTION_ATTR_USERSPACE action.
> +      value: 2
> +      attribute-set: packet
> +      event:
> +        attributes:

           attributes: *event-attrs

> +          - packet
> +          - key
> +          - userdata
> +          - actions
> +          - egress-tun-key
> +          - mru
> +          - len
> +          - hash

Otherwise looks good to me, thanks.

[PATCH 2/2] tools: ynl: add unicast notification receive support

[Minxi Hou]

Add ntf_listen() method to YnlFamily for binding the netlink
socket without joining a multicast group. This enables receiving
unicast notifications through the existing poll_ntf/check_ntf
path.

The OVS packet family sends MISS and ACTION upcalls via
genlmsg_unicast() to a per-vport PID rather than through a
multicast group. The existing ntf_subscribe() couples bind()
with setsockopt(ADD_MEMBERSHIP), which does not fit the unicast
case. ntf_listen() provides the bind-only alternative.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
 tools/net/ynl/pyynl/lib/ynl.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index f63c6f828735..8f526c683f99 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -748,6 +748,10 @@ class YnlFamily(SpecFamily):
         self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
                              mcast_id)
 
+    def ntf_listen(self):
+        """Bind socket for receiving unicast notifications."""
+        self.sock.bind((0, 0))
+
     def set_recv_dbg(self, enabled):
         self._recv_dbg = enabled
 
-- 
2.53.0

Re: [PATCH 2/2] tools: ynl: add unicast notification receive support

[Donald Hunter]

Minxi Hou <houminxi@gmail.com> writes:

> Add ntf_listen() method to YnlFamily for binding the netlink
> socket without joining a multicast group. This enables receiving
> unicast notifications through the existing poll_ntf/check_ntf
> path.
>
> The OVS packet family sends MISS and ACTION upcalls via
> genlmsg_unicast() to a per-vport PID rather than through a
> multicast group. The existing ntf_subscribe() couples bind()
> with setsockopt(ADD_MEMBERSHIP), which does not fit the unicast
> case. ntf_listen() provides the bind-only alternative.
>
> Signed-off-by: Minxi Hou <houminxi@gmail.com>
> ---
>  tools/net/ynl/pyynl/lib/ynl.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index f63c6f828735..8f526c683f99 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -748,6 +748,10 @@ class YnlFamily(SpecFamily):
>          self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
>                               mcast_id)
>  
> +    def ntf_listen(self):
> +        """Bind socket for receiving unicast notifications."""
> +        self.sock.bind((0, 0))

Can you add ntf_listen to the docstring for YnlFamily under the
"Notification API" heading.

> +
>      def set_recv_dbg(self, enabled):
>          self._recv_dbg = enabled

Re: [PATCH net-next 0/2] netlink: specs: complete OVS family coverage

[Minxi Hou]

Hi Aaron, Ilya,

I sent the first two patches of the pyroute2-to-ynl migration series.
Before continuing with patch 3 (the actual ovs-dpctl.py migration),
I wanted to check if the design direction matches what you had in mind.

A few questions:

  1. ovs_packet.yaml marks key/actions/egress-tun-key as type: binary
     instead of nest, since ynl doesn't support cross-spec attribute
     references.  The plan is to load ovs_flow's YnlFamily separately
     to decode those blobs.  Does that seem reasonable, or would you
     prefer duplicating the attribute-sets (~600 lines) into
     ovs_packet.yaml for full typing?

  2. For upcall receiving, patch 2 adds ntf_listen() to pyynl so we
     can bind a socket and receive unicast MISS/ACTION upcalls through
     the existing poll_ntf path.  The alternative would be raw socket
     code in the selftest.  Any preference?

  3. The remaining pyroute2 usage in ovs-dpctl.py is pyroute2.iproute
     (tunnel interface creation) and NDB (interface lookup).  I'm
     planning to replace those with subprocess calls to `ip`.  Any
     concerns with that approach?

  4. For the ODP string parser (~1500 lines of flow key/action
     encoding), that stays as-is since it handles the human-readable
     input format.  Anything there you'd want changed?

Happy to adjust the design before writing patch 3.

Thanks,
Minxi