Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest
@ 2026-10-09  6:06 Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 1/3] gtp: set a valid genl command in PDP context get and dump replies Anil Kaushik
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Anil Kaushik @ 2026-10-09  6:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Harald Welte, Donald Hunter, Jakub Kicinski,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Andrew Lunn, Shuah Khan
  Cc: osmocom-net-gprs, netdev, linux-kernel, linux-kselftest

This respins the RFC "netlink: specs: add genetlink-legacy spec for GTP"
(netdev, 2026-10-01) as a patch, and adds a driver fix the spec exposed
plus a selftest that uses it.

While writing a ynl-based selftest for the existing gtp family I found
that the GET and dump replies put the genl family id in the command
field instead of a GTP_CMD_* value. libgtpnl ignores the command so it
never mattered, but ynl rejects the reply as an unknown command and
drops it. Patch 1 fixes the driver; patches 2 and 3 then describe and
test the family.

Patch 1 sets a valid command (GTP_CMD_GETPDP) in the get and dump
replies.

Patch 2 adds a genetlink-legacy ynl spec for the gtp family
(NEWPDP/DELPDP/GETPDP/ECHOREQ). Description only, no uapi change.

Patch 3 adds a selftest built on the spec: it creates a gtp device, adds
a PDP context with newpdp, checks getpdp lists it and delpdp removes it.
It skips when the kernel or iproute2 lack GTP support.

Changes since v2:
 - selftest: add CONFIG_GTP to tools/testing/selftests/net/config
   (kept in sorted order).
 - selftest: move gtp.py to its correct alphabetical slot in the net
   Makefile (after the gre_* entries) and mark the script executable.
 - selftest: add docstrings to the three test functions and main();
   ruff check is clean under tools/testing/selftests/net/ruff.toml.
 - selftest: move GtpFamily before PSPFamily in lib/py (import and
   __all__).
 - Patches 1 and 2 are unchanged from v2.

v2: https://lore.kernel.org/netdev/20261008181031.4129029-1-anilkaushikwireless@gmail.com/

Anil Kaushik (3):
  gtp: set a valid genl command in PDP context get and dump replies
  netlink: specs: add genetlink-legacy spec for GTP
  selftests: net: add a test for the gtp netlink family

 Documentation/netlink/specs/gtp.yaml          | 170 ++++++++++++++++++
 MAINTAINERS                                   |   2 +
 drivers/net/gtp.c                             |   4 +-
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/gtp.py            |  85 +++++++++
 .../testing/selftests/net/lib/py/__init__.py  |   5 +-
 tools/testing/selftests/net/lib/py/ynl.py     |   8 +-
 8 files changed, 271 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/netlink/specs/gtp.yaml
 create mode 100755 tools/testing/selftests/net/gtp.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v3 1/3] gtp: set a valid genl command in PDP context get and dump replies
  2026-10-09  6:06 [PATCH net-next v3 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest Anil Kaushik
@ 2026-10-09  6:06 ` Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family Anil Kaushik
  2 siblings, 0 replies; 7+ messages in thread
From: Anil Kaushik @ 2026-10-09  6:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Harald Welte, Donald Hunter, Jakub Kicinski,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Andrew Lunn, Shuah Khan
  Cc: osmocom-net-gprs, netdev, linux-kernel, linux-kselftest

gtp_genl_fill_info() stamps the reply with the command its callers pass
in. gtp_genl_get_pdp() and gtp_genl_dump_pdp() pass nlmsg_type, which in
a genl message is the dynamically assigned family id rather than a
GTP_CMD_* value, so replies to a PDP context get or dump carry a
meaningless command number.

libgtpnl ignores the command and is unaffected, but parsers that check
it (for example the ynl tooling) treat the reply as an unknown command
and discard it. Pass GTP_CMD_GETPDP, as gtp_tunnel_notify() already
passes a real command on the notification path. No uapi change.

Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 drivers/net/gtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4aff23bcfdd1..99836b773e5c 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -2279,7 +2279,7 @@ static int gtp_genl_get_pdp(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	err = gtp_genl_fill_info(skb2, NETLINK_CB(skb).portid, info->snd_seq,
-				 0, info->nlhdr->nlmsg_type, pctx);
+				 0, GTP_CMD_GETPDP, pctx);
 	if (err < 0)
 		goto err_unlock_free;
 
@@ -2326,7 +2326,7 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 					    NETLINK_CB(cb->skb).portid,
 					    cb->nlh->nlmsg_seq,
 					    NLM_F_MULTI,
-					    cb->nlh->nlmsg_type, pctx)) {
+					    GTP_CMD_GETPDP, pctx)) {
 					cb->args[0] = i;
 					cb->args[1] = j;
 					cb->args[2] = (unsigned long)gtp;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP
  2026-10-09  6:06 [PATCH net-next v3 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 1/3] gtp: set a valid genl command in PDP context get and dump replies Anil Kaushik
@ 2026-10-09  6:06 ` Anil Kaushik
  2026-10-10  6:21   ` netdev-bot+sashiko
  2026-10-09  6:06 ` [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family Anil Kaushik
  2 siblings, 1 reply; 7+ messages in thread
From: Anil Kaushik @ 2026-10-09  6:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Harald Welte, Donald Hunter, Jakub Kicinski,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Andrew Lunn, Shuah Khan
  Cc: osmocom-net-gprs, netdev, linux-kernel, linux-kselftest

The GTP (GPRS Tunnelling Protocol, user plane) generic netlink family
has no YAML specification under Documentation/netlink/specs/, so it
cannot be consumed by the ynl tooling used for user-space clients,
documentation and selftests.

Add a genetlink-legacy spec describing the existing family: the PDP
context management commands (NEWPDP, DELPDP, GETPDP) and the GTP-U echo
request (ECHOREQ), the GTPA_* attribute set, and the "gtp" multicast
group. The spec is derived directly from include/uapi/linux/gtp.h and
the gtp_genl_policy / gtp_genl_ops tables in drivers/net/gtp.c; command
and attribute values match the existing uapi one-to-one.

This only adds the description; there is no kernel code or uapi change.

Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 Documentation/netlink/specs/gtp.yaml | 170 +++++++++++++++++++++++++++
 MAINTAINERS                          |   1 +
 2 files changed, 171 insertions(+)
 create mode 100644 Documentation/netlink/specs/gtp.yaml

diff --git a/Documentation/netlink/specs/gtp.yaml b/Documentation/netlink/specs/gtp.yaml
new file mode 100644
index 000000000000..7193f5e53a25
--- /dev/null
+++ b/Documentation/netlink/specs/gtp.yaml
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+---
+name: gtp
+
+protocol: genetlink-legacy
+
+doc: |
+  GPRS Tunnelling Protocol, user plane (GTP-U).
+
+  The gtp netdevice encapsulates and decapsulates user plane packets in
+  GTP-U tunnels (GTPv0 and GTPv1-U, see 3GPP TS 29.060 and TS 29.281).
+  This family manages the PDP contexts that describe the tunnels and
+  triggers GTP-U echo requests. It is driven by user space control planes
+  such as those built on libgtpnl.
+
+kernel-policy: global
+
+attribute-sets:
+  -
+    name: gtp
+    name-prefix: gtpa-
+    attributes:
+      -
+        name: link
+        type: u32
+        doc: ifindex of the gtp netdevice the context is attached to.
+      -
+        name: version
+        type: u32
+        doc: GTP version of the context, 0 for GTPv0 or 1 for GTPv1-U.
+      -
+        name: tid
+        type: u64
+        doc: Tunnel identifier, GTPv0 only.
+      -
+        name: peer-address
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+        doc: |
+          IPv4 address of the remote GSN peer (GGSN or SGSN). Also known
+          as GTPA_SGSN_ADDRESS, kept for legacy user space.
+      -
+        name: ms-address
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+        doc: IPv4 address of the mobile subscriber served by the context.
+      -
+        name: flow
+        type: u16
+        doc: Flow label, GTPv0 only.
+      -
+        name: net-ns-fd
+        type: u32
+        doc: File descriptor of the network namespace of the gtp netdevice.
+      -
+        name: i-tei
+        type: u32
+        doc: Ingress Tunnel Endpoint Identifier, GTPv1-U only.
+      -
+        name: o-tei
+        type: u32
+        doc: Egress Tunnel Endpoint Identifier, GTPv1-U only.
+      -
+        name: pad
+        type: pad
+      -
+        name: peer-addr6
+        type: binary
+        checks:
+          exact-len: 16
+        byte-order: big-endian
+        display-hint: ipv6
+        doc: IPv6 address of the remote GSN peer (GGSN or SGSN).
+      -
+        name: ms-addr6
+        type: binary
+        checks:
+          exact-len: 16
+        byte-order: big-endian
+        display-hint: ipv6
+        doc: IPv6 address of the mobile subscriber served by the context.
+      -
+        name: family
+        type: u8
+        doc: Address family (AF_INET or AF_INET6) of the context addresses.
+
+operations:
+  list:
+    -
+      name: newpdp
+      doc: Create or update a PDP context.
+      attribute-set: gtp
+      value: 0
+      dont-validate: [strict, dump]
+      flags: [admin-perm]
+      do:
+        request: &pdp-attrs
+          attributes:
+            - link
+            - version
+            - tid
+            - peer-address
+            - peer-addr6
+            - ms-address
+            - ms-addr6
+            - flow
+            - i-tei
+            - o-tei
+            - family
+            - net-ns-fd
+    -
+      name: delpdp
+      doc: Delete a PDP context.
+      attribute-set: gtp
+      dont-validate: [strict, dump]
+      flags: [admin-perm]
+      do:
+        request: *pdp-attrs
+    -
+      name: getpdp
+      doc: Get or dump one or more PDP contexts.
+      attribute-set: gtp
+      dont-validate: [strict, dump]
+      flags: [admin-perm]
+      do:
+        request:
+          attributes:
+            - link
+            - version
+            - tid
+            - ms-address
+            - ms-addr6
+            - i-tei
+            - family
+            - net-ns-fd
+        reply: &pdp-reply
+          attributes:
+            - version
+            - tid
+            - peer-address
+            - peer-addr6
+            - ms-address
+            - ms-addr6
+            - flow
+            - i-tei
+            - o-tei
+            - family
+      dump:
+        reply: *pdp-reply
+    -
+      name: echoreq
+      doc: Send a GTP-U echo request to a peer.
+      attribute-set: gtp
+      dont-validate: [strict, dump]
+      flags: [admin-perm]
+      do:
+        request:
+          attributes:
+            - link
+            - version
+            - peer-address
+            - peer-addr6
+            - family
+
+mcast-groups:
+  list:
+    -
+      name: gtp
diff --git a/MAINTAINERS b/MAINTAINERS
index 51873349ba91..a6e43995ea58 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11421,6 +11421,7 @@ M:	Harald Welte <laforge@gnumonks.org>
 L:	osmocom-net-gprs@lists.osmocom.org
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git
+F:	Documentation/netlink/specs/gtp.yaml
 F:	drivers/net/gtp.c
 
 GUID PARTITION TABLE (GPT)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family
  2026-10-09  6:06 [PATCH net-next v3 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 1/3] gtp: set a valid genl command in PDP context get and dump replies Anil Kaushik
  2026-10-09  6:06 ` [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP Anil Kaushik
@ 2026-10-09  6:06 ` Anil Kaushik
  2026-10-10  6:21   ` netdev-bot+sashiko
  2 siblings, 1 reply; 7+ messages in thread
From: Anil Kaushik @ 2026-10-09  6:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Harald Welte, Donald Hunter, Jakub Kicinski,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Andrew Lunn, Shuah Khan
  Cc: osmocom-net-gprs, netdev, linux-kernel, linux-kselftest

Exercise the gtp generic netlink family through its ynl spec: create a
gtp device, add a PDP context with newpdp, check getpdp lists it and
delpdp removes it. Two more cases check that two contexts are dumped and
that deleting an unknown context fails.

Add a GtpFamily ynl wrapper and register the test.

Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 MAINTAINERS                                   |  1 +
 tools/testing/selftests/net/Makefile          |  1 +
 tools/testing/selftests/net/config            |  1 +
 tools/testing/selftests/net/gtp.py            | 85 +++++++++++++++++++
 .../testing/selftests/net/lib/py/__init__.py  |  5 +-
 tools/testing/selftests/net/lib/py/ynl.py     |  8 +-
 6 files changed, 98 insertions(+), 3 deletions(-)
 create mode 100755 tools/testing/selftests/net/gtp.py

diff --git a/MAINTAINERS b/MAINTAINERS
index a6e43995ea58..8ac817a78bb0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11423,6 +11423,7 @@ S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git
 F:	Documentation/netlink/specs/gtp.yaml
 F:	drivers/net/gtp.c
+F:	tools/testing/selftests/net/gtp.py
 
 GUID PARTITION TABLE (GPT)
 M:	Davidlohr Bueso <dave@stgolabs.net>
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index d4ca82fec0b4..6b54e04a6430 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -46,6 +46,7 @@ TEST_PROGS := \
 	fq_band_pktlimit.sh \
 	gre_gso.sh \
 	gre_ipv6_lladdr.sh \
+	gtp.py \
 	icmp.sh \
 	icmp_redirect.sh \
 	io_uring_zerocopy_tx.sh \
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 737e7e6327b3..c4534e15fb7a 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -15,6 +15,7 @@ CONFIG_DEBUG_INFO_BTF=y
 CONFIG_DEBUG_INFO_BTF_MODULES=n
 CONFIG_DUMMY=y
 CONFIG_GENEVE=m
+CONFIG_GTP=m
 CONFIG_IFB=y
 CONFIG_INET_DIAG=y
 CONFIG_INET_ESP=y
diff --git a/tools/testing/selftests/net/gtp.py b/tools/testing/selftests/net/gtp.py
new file mode 100755
index 000000000000..fcc9bf57a663
--- /dev/null
+++ b/tools/testing/selftests/net/gtp.py
@@ -0,0 +1,85 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Tests for the gtp netlink family."""
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, ksft_in, ksft_not_in, ksft_raises
+from lib.py import KsftSkipEx
+from lib.py import NetNS, NetNSEnter
+from lib.py import GtpFamily, NlError
+from lib.py import CmdExitFailure
+from lib.py import ip, defer
+
+
+def _add_gtp_dev(ns):
+    try:
+        ip("link add gtp0 type gtp role ggsn", ns=str(ns))
+    except CmdExitFailure:
+        raise KsftSkipEx("no gtp support (CONFIG_GTP, iproute2)")
+    defer(ip, "link del gtp0", ns=str(ns))
+    ip("link set gtp0 up", ns=str(ns))
+    return ip("-d link show gtp0", ns=str(ns), json=True)[0]["ifindex"]
+
+
+def _iteis(contexts):
+    return [c.get("i-tei") for c in contexts]
+
+
+def add_del_pdp(gtp, ns) -> None:
+    """Add a PDP context, query it, then delete it.
+
+    The query must return the context with the TEIDs and addresses
+    it was created with, and it must be gone after the delete.
+    """
+    link = _add_gtp_dev(ns)
+
+    gtp.newpdp({"link": link, "version": 1,
+                "i-tei": 0x100, "o-tei": 0x200,
+                "ms-address": "10.0.0.1", "peer-address": "192.0.2.1"})
+
+    pdp = [c for c in gtp.getpdp({}, dump=True) if c.get("i-tei") == 0x100]
+    ksft_eq(len(pdp), 1)
+    ksft_eq(pdp[0]["o-tei"], 0x200)
+    ksft_eq(pdp[0]["ms-address"], "10.0.0.1")
+    ksft_eq(pdp[0]["peer-address"], "192.0.2.1")
+
+    gtp.delpdp({"link": link, "version": 1,
+                "i-tei": 0x100, "ms-address": "10.0.0.1"})
+    ksft_not_in(0x100, _iteis(gtp.getpdp({}, dump=True)))
+
+
+def add_two_pdp(gtp, ns) -> None:
+    """Add two PDP contexts and check both are returned."""
+    link = _add_gtp_dev(ns)
+
+    for i_tei, o_tei, ms in ((0x111, 0x211, "10.0.1.1"),
+                             (0x112, 0x212, "10.0.1.2")):
+        gtp.newpdp({"link": link, "version": 1,
+                    "i-tei": i_tei, "o-tei": o_tei,
+                    "ms-address": ms, "peer-address": "192.0.2.5"})
+
+    iteis = _iteis(gtp.getpdp({}, dump=True))
+    ksft_in(0x111, iteis)
+    ksft_in(0x112, iteis)
+
+
+def del_unknown_pdp(gtp, ns) -> None:
+    """Deleting a context that was never added must fail."""
+    link = _add_gtp_dev(ns)
+    with ksft_raises(NlError):
+        gtp.delpdp({"link": link, "version": 1,
+                    "i-tei": 0x999, "ms-address": "10.9.9.9"})
+
+
+def main() -> None:
+    """Run the PDP context tests in a temporary netns."""
+    with NetNS() as ns:
+        with NetNSEnter(str(ns)):
+            gtp = GtpFamily()
+        ksft_run([add_del_pdp, add_two_pdp, del_unknown_pdp], args=(gtp, ns))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index 71df5880b356..e269a260d58e 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -18,7 +18,7 @@ from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
 from .bpf import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
 from .ynl import NlError, NlctrlFamily, YnlFamily, \
     EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily, RtnlRouteFamily
-from .ynl import NetshaperFamily, DevlinkFamily, PSPFamily, Netlink
+from .ynl import NetshaperFamily, DevlinkFamily, GtpFamily, PSPFamily, Netlink
 
 __all__ = ["KSRC",
            "KsftFailEx", "KsftSkipEx", "KsftXfailEx", "ksft_pr", "ksft_eq",
@@ -32,6 +32,7 @@ __all__ = ["KSRC",
            "wait_port_listen", "ctl_file_write", "wait_file", "tool", "tc",
            "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
            "NetdevSim", "NetdevSimDev",
-           "NetshaperFamily", "DevlinkFamily", "PSPFamily", "NlError",
+           "NetshaperFamily", "DevlinkFamily", "GtpFamily", "PSPFamily",
+           "NlError",
            "YnlFamily", "EthtoolFamily", "NetdevFamily", "RtnlFamily",
            "NlctrlFamily", "RtnlAddrFamily", "RtnlRouteFamily", "Netlink"]
diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py
index 08deff756f29..7edb74971023 100644
--- a/tools/testing/selftests/net/lib/py/ynl.py
+++ b/tools/testing/selftests/net/lib/py/ynl.py
@@ -31,7 +31,7 @@ __all__ = [
     "NlError", "NlPolicy", "Netlink", "YnlFamily", "SPEC_PATH",
     "EthtoolFamily", "RtnlFamily", "RtnlAddrFamily", "RtnlRouteFamily",
     "NetdevFamily", "NetshaperFamily", "NlctrlFamily", "DevlinkFamily",
-    "PSPFamily",
+    "GtpFamily", "PSPFamily",
 ]
 
 #
@@ -85,3 +85,9 @@ class PSPFamily(YnlFamily):
     def __init__(self, recv_size=0):
         super().__init__((SPEC_PATH / Path('psp.yaml')).as_posix(),
                          schema='', recv_size=recv_size)
+
+
+class GtpFamily(YnlFamily):
+    def __init__(self, recv_size=0):
+        super().__init__((SPEC_PATH / Path('gtp.yaml')).as_posix(),
+                         schema='', recv_size=recv_size)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP
  2026-10-09  6:06 ` [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP Anil Kaushik
@ 2026-10-10  6:21   ` netdev-bot+sashiko
  0 siblings, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-10-10  6:21 UTC (permalink / raw)
  To: anilkaushikwireless
  Cc: pablo, laforge, donald.hunter, kuba, davem, edumazet, pabeni,
	horms, andrew+netdev, shuah, osmocom-net-gprs, netdev,
	linux-kernel, linux-kselftest

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 4

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The echoreq request attribute list in
  Documentation/netlink/specs/gtp.yaml (lines 159-165) is wrong.
- [Medium] The getpdp reply attribute list (&pdp-reply,
  Documentation/netlink/specs/gtp.yaml lines 138-149) leaves out link.
- [Medium] The spec declares the 'gtp' multicast group (lines 167-170),
  but no operation has a notify, event or mcgrp entry.
- [Low] The spec gives peer-addr6 and ms-addr6 'checks: exact-len: 16'
  (gtp.yaml lines 71-72 and 79-80).
- [Low] The delpdp request reuses the full newpdp attribute list
  ('request: *pdp-attrs', gtp.yaml line 120).
- [Low] The new spec has no CFLAGS_gtp entry in
  tools/net/ynl/Makefile.deps.
- [Low] The spec leaves out the top-level 'version' property, so the
  genetlink-legacy default of 1 applies (genetlink-legacy.yaml: 'Default…

--

netlink: specs: add genetlink-legacy spec for GTP

This commit adds a genetlink-legacy YAML spec for the existing GTP generic
netlink family. The spec covers the NEWPDP, DELPDP, GETPDP and ECHOREQ
commands, the GTPA_* attribute set and the "gtp" multicast group. It also
adds the new spec file to the GTP entry in MAINTAINERS.

> diff --git a/Documentation/netlink/specs/gtp.yaml b/Documentation/netlink/specs/gtp.yaml
> new file mode 100644
> index 0000000000000..7193f5e53a257
> --- /dev/null
> +++ b/Documentation/netlink/specs/gtp.yaml
> @@ -0,0 +1,170 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +---
> +name: gtp
> +
> +protocol: genetlink-legacy

[Severity: Low]
This spec has no version property, so the genetlink-legacy schema default
of 1 applies. The kernel registers the family with version 0:

drivers/net/gtp.c:
static struct genl_family gtp_genl_family __ro_after_init = {
	.name		= "gtp",
	.version	= 0,

The schema in Documentation/netlink/genetlink-legacy.yaml also says
"minimum: 1". That means the spec can't state the real value unless the
schema is relaxed.

The runtime effect seems limited. Both the C and Python ynl code hard-code
version 1 in requests, and the kernel does not check the request version.
The remaining problem is wrong metadata, such as a GTP_FAMILY_VERSION of 1
if uapi or kernel code were ever generated from this spec.

Should the spec record version 0, with the schema relaxed to allow it?

[ ... ]

> +      -
> +        name: peer-addr6
> +        type: binary
> +        checks:
> +          exact-len: 16
> +        byte-order: big-endian
> +        display-hint: ipv6
> +        doc: IPv6 address of the remote GSN peer (GGSN or SGSN).

[Severity: Low]
Does exact-len match the kernel policy? gtp_genl_policy has:

	[GTPA_PEER_ADDR6]	= { .len = sizeof(struct in6_addr), },
	[GTPA_MS_ADDR6]		= { .len = sizeof(struct in6_addr), },

These entries have no type, so they are NLA_UNSPEC, and .len is only a
minimum length. Every gtp op sets GENL_DONT_VALIDATE_STRICT, so the kernel
accepts attributes longer than 16 bytes. The spec describes those as
invalid.

Should this be min-len: 16 here and for ms-addr6? Alternatively, the
policy could switch to NLA_POLICY_EXACT_LEN.

[ ... ]

> +    -
> +      name: delpdp
> +      doc: Delete a PDP context.
> +      attribute-set: gtp
> +      dont-validate: [strict, dump]
> +      flags: [admin-perm]
> +      do:
> +        request: *pdp-attrs

[Severity: Low]
This reuses the full newpdp attribute list. The delete path is
gtp_genl_del_pdp()->gtp_find_pdp()->gtp_find_pdp_by_link()/gtp_find_dev(),
and it only reads link, net-ns-fd, family, ms-address, ms-addr6, version,
tid and i-tei.

peer-address, peer-addr6, flow and o-tei are ignored, but ynl will still
generate setters for them.

Would the lookup-only list used by the getpdp request be a better fit for
delpdp?

> +    -
> +      name: getpdp
> +      doc: Get or dump one or more PDP contexts.

[ ... ]

> +        reply: &pdp-reply
> +          attributes:
> +            - version
> +            - tid
> +            - peer-address
> +            - peer-addr6
> +            - ms-address
> +            - ms-addr6
> +            - flow
> +            - i-tei
> +            - o-tei
> +            - family
> +      dump:
> +        reply: *pdp-reply

[Severity: Medium]
Should link be part of this reply list? gtp_genl_fill_info() always emits
it:

	if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
	    nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
	    nla_put_u8(skb, GTPA_FAMILY, pctx->af))

The C parser generated by ynl_gen_c only decodes the listed reply
attributes. That means link is dropped from getpdp do replies, from dump
entries and from NEWPDP/DELPDP notifications.

A dump walks every gtp device in the netns. A generated C client therefore
can't tell which device a context belongs to. It also can't build the
matching delpdp or getpdp request, because gtp_find_pdp() needs GTPA_LINK.

The Python ynl used by selftests decodes against the whole attribute set,
so it would not catch this.

> +    -
> +      name: echoreq
> +      doc: Send a GTP-U echo request to a peer.
> +      attribute-set: gtp
> +      dont-validate: [strict, dump]
> +      flags: [admin-perm]
> +      do:
> +        request:
> +          attributes:
> +            - link
> +            - version
> +            - peer-address
> +            - peer-addr6
> +            - family

[Severity: Medium]
Can a generated client ever get a successful echoreq with this list?
gtp_genl_send_echo_req() requires ms-address and uses it as the source
address:

	if (!info->attrs[GTPA_VERSION] ||
	    !info->attrs[GTPA_LINK] ||
	    !info->attrs[GTPA_PEER_ADDRESS] ||
	    !info->attrs[GTPA_MS_ADDRESS])
		return -EINVAL;
	...
	src_ip = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);

ynl_gen_c builds the request struct only from the op's listed attributes,
in RenderInfo:

	type_list = op[op_mode][op_dir]['attributes']

So the generated C echoreq request has no way to set ms-address, and
every call returns -EINVAL.

The handler looks up the device through gtp_find_dev(), which reads
GTPA_NET_NS_FD when it is present. net-ns-fd isn't listed either, so a
gtp device in another netns can't be targeted.

peer-addr6 and family are listed, but the handler never reads them because
echo requests are IPv4 only. A request with only an IPv6 peer in
peer-addr6 would still get -EINVAL.

Should this list be link, version, peer-address, ms-address and net-ns-fd?

> +
> +mcast-groups:
> +  list:
> +    -
> +      name: gtp

[Severity: Medium]
The gtp group is declared, but no operation has a notify or event entry
that describes what the kernel sends on it. The kernel multicasts three
kinds of message on GTP_GENL_MCGRP:

- GTP_CMD_NEWPDP from gtp_genl_new_pdp() via gtp_tunnel_notify()
- GTP_CMD_DELPDP from gtp_genl_del_pdp() via gtp_tunnel_notify(), both
  carrying the gtp_genl_fill_info() contents
- GTP_CMD_ECHOREQ when an echo response arrives, from
  gtp0_handle_echo_resp() and its GTPv1-U counterpart

The echo response case looks like this:

	ret = gtp_genl_fill_echo(msg, 0, 0, 0, GTP_CMD_ECHOREQ, echo);
	...
	return genlmsg_multicast_netns(&gtp_genl_family, dev_net(gtp->dev),
				       msg, 0, GTP_GENL_MCGRP, GFP_ATOMIC);

In tools/net/ynl/pyynl/lib/nlspec.py, SpecOperation marks an op as async
only when 'notify' or 'event' is present. YnlFamily.check_ntf() drops
everything else:

    if decoded.cmd() not in self.async_msg_ids:
        print("Unexpected msg id while checking for ntf", decoded)
        continue

A ynl client subscribed to gtp would throw away every notification. That
includes the echo response report, which is the only way to get the
result of an echoreq. ynl_gen_c also generates no notification parsers
for this family.

The notifications reuse the request command IDs, so describing them may
need explicit values or a schema change. Should the spec describe these
messages in some form?

[Severity: Low]
Is a tools/net/ynl/Makefile.deps entry needed for this new spec?
tools/net/ynl/generated/Makefile builds every spec under
Documentation/netlink/specs except those in GENS_UNSUP, so gtp-user.c is
now generated and compiled.

The generated code includes <linux/gtp.h>. The in-tree uapi directory is
only added with -idirafter, so the host's installed header is found first.
Hosts with older headers lack GTPA_PEER_ADDR6, GTPA_MS_ADDR6 and
GTPA_FAMILY, and very old ones also lack GTP_CMD_ECHOREQ. The ynl build
would fail on those hosts.

Other families handle this with an entry like:

CFLAGS_ovpn:=$(call get_hdr_inc,_LINUX_OVPN_H,ovpn.h)
CFLAGS_psp:=$(call get_hdr_inc,_LINUX_PSP_H,psp.h)

Would something like this be needed for gtp?

CFLAGS_gtp:=$(call get_hdr_inc,_LINUX_GTP_H_,gtp.h)

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009060615.4177402-1-anilkaushikwireless%40gmail.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family
  2026-10-09  6:06 ` [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family Anil Kaushik
@ 2026-10-10  6:21   ` netdev-bot+sashiko
  2026-10-10 10:56     ` Anil Kaushik
  0 siblings, 1 reply; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-10-10  6:21 UTC (permalink / raw)
  To: anilkaushikwireless
  Cc: pablo, laforge, donald.hunter, kuba, davem, edumazet, pabeni,
	horms, andrew+netdev, shuah, osmocom-net-gprs, netdev,
	linux-kernel, linux-kselftest

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new test aborts with a traceback instead of running or
  skipping whenever the 'gtp' genl family is not already registered.
- [Medium] The `echoreq` operation in
  Documentation/netlink/specs/gtp.yaml (lines 159-165) declares its…
- [Low] del_unknown_pdp() in tools/testing/selftests/net/gtp.py uses
  `with ksft_raises(NlError)` without checking the errno, so any netlink…
- [Low] The getpdp `&pdp-reply` attribute set in
  Documentation/netlink/specs/gtp.yaml (lines 138-149, used by getpdp do…

--

selftests: net: add a test for the gtp netlink family

This adds a Python selftest that drives the gtp generic netlink family
through its ynl spec. It covers newpdp, getpdp and delpdp, a two-context
dump, and a delete of an unknown context. It also adds a GtpFamily ynl
wrapper, CONFIG_GTP=m in the net selftest config, and Makefile and
MAINTAINERS entries.

> diff --git a/tools/testing/selftests/net/gtp.py b/tools/testing/selftests/net/gtp.py
> new file mode 100755
> index 0000000000000..fcc9bf57a663f
> --- /dev/null
> +++ b/tools/testing/selftests/net/gtp.py
> @@ -0,0 +1,85 @@

[ ... ]

> +def add_del_pdp(gtp, ns) -> None:
> +    """Add a PDP context, query it, then delete it.
> +
> +    The query must return the context with the TEIDs and addresses
> +    it was created with, and it must be gone after the delete.
> +    """
> +    link = _add_gtp_dev(ns)
> +
> +    gtp.newpdp({"link": link, "version": 1,
> +                "i-tei": 0x100, "o-tei": 0x200,
> +                "ms-address": "10.0.0.1", "peer-address": "192.0.2.1"})
> +
> +    pdp = [c for c in gtp.getpdp({}, dump=True) if c.get("i-tei") == 0x100]

[Severity: Low]
This dump is decoded with Documentation/netlink/specs/gtp.yaml. That spec
was added earlier in this series by "netlink: specs: add genetlink-legacy
spec for GTP", and its getpdp reply leaves out link:

        reply: &pdp-reply
          attributes:
            - version
            - tid
            - peer-address
            - peer-addr6
            - ms-address
            - ms-addr6
            - flow
            - i-tei
            - o-tei
            - family

But gtp_genl_fill_info() always emits it, in every getpdp reply, dump entry
and notification:

	if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
	    nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
	    nla_put_u8(skb, GTPA_FAMILY, pctx->af))

Should link be added to the reply attributes? Without it, consumers
generated from the spec can't tell which gtp device a dumped context
belongs to.

On a related note, this filter matches only on i-tei, not on the link the
test just created.

> +    ksft_eq(len(pdp), 1)
> +    ksft_eq(pdp[0]["o-tei"], 0x200)
> +    ksft_eq(pdp[0]["ms-address"], "10.0.0.1")
> +    ksft_eq(pdp[0]["peer-address"], "192.0.2.1")
> +
> +    gtp.delpdp({"link": link, "version": 1,
> +                "i-tei": 0x100, "ms-address": "10.0.0.1"})
> +    ksft_not_in(0x100, _iteis(gtp.getpdp({}, dump=True)))

[ ... ]

> +def del_unknown_pdp(gtp, ns) -> None:
> +    """Deleting a context that was never added must fail."""
> +    link = _add_gtp_dev(ns)
> +    with ksft_raises(NlError):
> +        gtp.delpdp({"link": link, "version": 1,
> +                    "i-tei": 0x999, "ms-address": "10.9.9.9"})

[Severity: Low]
Should this check the errno too? ksft_raises(NlError) accepts any netlink
error. For example, -EINVAL or -ENODEV from gtp_find_pdp_by_link() or
gtp_genl_del_pdp() would also pass. The real lookup miss in gtp_find_pdp()
is -ENOENT.

Also, both delpdp requests in this test send ms-address, and
gtp_find_pdp_by_link() checks that before the TEI branch:

drivers/net/gtp.c:gtp_find_pdp_by_link() {
    ...
	if (nla[GTPA_MS_ADDRESS]) {
		__be32 ip = nla_get_be32(nla[GTPA_MS_ADDRESS]);
		if (family != AF_INET)
			return ERR_PTR(-EINVAL);
		return ipv4_pdp_find(gtp, ip);
    ...
}

That means the i-tei values passed to delpdp are ignored, and delete by
TEI through gtp1_pdp_find() is never tested.

With the current kernel this request does return -ENOENT, so the test
passes for the right reason today. It just checks less than the docstring
suggests.

> +
> +
> +def main() -> None:
> +    """Run the PDP context tests in a temporary netns."""
> +    with NetNS() as ns:
> +        with NetNSEnter(str(ns)):
> +            gtp = GtpFamily()
> +        ksft_run([add_del_pdp, add_two_pdp, del_unknown_pdp], args=(gtp, ns))

[Severity: Medium]
Can GtpFamily() succeed here if gtp.ko isn't loaded yet?

GtpFamily() -> YnlFamily.__init__() -> GenlProtocol('gtp') looks up the
family id with _genl_load_families(). That sends CTRL_CMD_GETFAMILY with
NLM_F_DUMP, which the kernel answers from ctrl_dumpfamily(). Only the
non-dump ctrl_getfamily() path autoloads the module:

net/netlink/genetlink.c:ctrl_getfamily() {
    ...
	request_module("net-pf-%d-proto-%d-family-%s",
		       PF_NETLINK, NETLINK_GENERIC, name);
    ...
}

This patch adds CONFIG_GTP=m to the selftest config. On a fresh boot with
that config, the "gtp" family isn't registered yet, so
genl_family_name_to_id['gtp'] raises KeyError. YnlFamily.__init__()
turns that into:

    YnlException(f"Family '{self.yaml['name']}' not supported by the kernel")

That exception isn't a KsftSkipEx and is raised outside ksft_run(). Won't
the whole script abort with a traceback before any case runs?

gtp.ko would only be autoloaded later, through the rtnl link alias, when
_add_gtp_dev() runs "ip link add gtp0 type gtp". For the same reason, with
CONFIG_GTP unset the KsftSkipEx in _add_gtp_dev() is never reached, and the
test fails instead of skipping.

Would it work to create the gtp device, or otherwise load the module,
before constructing GtpFamily(), and to treat a missing family as a skip?

> +    ksft_exit()
> +
> +
> +if __name__ == "__main__":
> +    main()

[ ... ]

> diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py
> index 08deff756f29b..7edb749710231 100644
> --- a/tools/testing/selftests/net/lib/py/ynl.py
> +++ b/tools/testing/selftests/net/lib/py/ynl.py

[ ... ]

> @@ -85,3 +85,9 @@ class PSPFamily(YnlFamily):
>      def __init__(self, recv_size=0):
>          super().__init__((SPEC_PATH / Path('psp.yaml')).as_posix(),
>                           schema='', recv_size=recv_size)
> +
> +
> +class GtpFamily(YnlFamily):
> +    def __init__(self, recv_size=0):
> +        super().__init__((SPEC_PATH / Path('gtp.yaml')).as_posix(),
> +                         schema='', recv_size=recv_size)

[Severity: Medium]
This wrapper loads the gtp.yaml added earlier in this series by
"netlink: specs: add genetlink-legacy spec for GTP". Does that spec's
echoreq request match the driver? The spec lists:

        request:
          attributes:
            - link
            - version
            - peer-address
            - peer-addr6
            - family

gtp_genl_send_echo_req() requires GTPA_MS_ADDRESS, which isn't listed:

	if (!info->attrs[GTPA_VERSION] ||
	    !info->attrs[GTPA_LINK] ||
	    !info->attrs[GTPA_PEER_ADDRESS] ||
	    !info->attrs[GTPA_MS_ADDRESS])
		return -EINVAL;

The handler is also IPv4-only and never reads GTPA_PEER_ADDR6 or
GTPA_FAMILY.

A client generated from the spec would never send ms-address, so its
echoreq would always fail with -EINVAL.

That spec commit says it is "derived directly from include/uapi/linux/gtp.h
and the gtp_genl_policy / gtp_genl_ops tables in drivers/net/gtp.c". That
doesn't seem to hold for echoreq.

Should ms-address be added to the echoreq request, and peer-addr6 and
family removed?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009060615.4177402-1-anilkaushikwireless%40gmail.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family
  2026-10-10  6:21   ` netdev-bot+sashiko
@ 2026-10-10 10:56     ` Anil Kaushik
  0 siblings, 0 replies; 7+ messages in thread
From: Anil Kaushik @ 2026-10-10 10:56 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: pablo, laforge, donald.hunter, kuba, davem, edumazet, pabeni,
	horms, andrew+netdev, shuah, osmocom-net-gprs, netdev,
	linux-kernel, linux-kselftest

Thanks, all four are valid and fixed in v4
The echoreq request now matches gtp_genl_send_echo_req() and link is
in the getpdp reply
The test creates the gtp device before the family handle (and skips
without GTP).
And del_unknown_pdp checks ENOENT for both the TEI and MS-address lookups.

On Sat, Oct 10, 2026 at 11:51 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 2
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The new test aborts with a traceback instead of running or
>   skipping whenever the 'gtp' genl family is not already registered.
> - [Medium] The `echoreq` operation in
>   Documentation/netlink/specs/gtp.yaml (lines 159-165) declares its…
> - [Low] del_unknown_pdp() in tools/testing/selftests/net/gtp.py uses
>   `with ksft_raises(NlError)` without checking the errno, so any netlink…
> - [Low] The getpdp `&pdp-reply` attribute set in
>   Documentation/netlink/specs/gtp.yaml (lines 138-149, used by getpdp do…
>
> --
>
> selftests: net: add a test for the gtp netlink family
>
> This adds a Python selftest that drives the gtp generic netlink family
> through its ynl spec. It covers newpdp, getpdp and delpdp, a two-context
> dump, and a delete of an unknown context. It also adds a GtpFamily ynl
> wrapper, CONFIG_GTP=m in the net selftest config, and Makefile and
> MAINTAINERS entries.
>
> > diff --git a/tools/testing/selftests/net/gtp.py b/tools/testing/selftests/net/gtp.py
> > new file mode 100755
> > index 0000000000000..fcc9bf57a663f
> > --- /dev/null
> > +++ b/tools/testing/selftests/net/gtp.py
> > @@ -0,0 +1,85 @@
>
> [ ... ]
>
> > +def add_del_pdp(gtp, ns) -> None:
> > +    """Add a PDP context, query it, then delete it.
> > +
> > +    The query must return the context with the TEIDs and addresses
> > +    it was created with, and it must be gone after the delete.
> > +    """
> > +    link = _add_gtp_dev(ns)
> > +
> > +    gtp.newpdp({"link": link, "version": 1,
> > +                "i-tei": 0x100, "o-tei": 0x200,
> > +                "ms-address": "10.0.0.1", "peer-address": "192.0.2.1"})
> > +
> > +    pdp = [c for c in gtp.getpdp({}, dump=True) if c.get("i-tei") == 0x100]
>
> [Severity: Low]
> This dump is decoded with Documentation/netlink/specs/gtp.yaml. That spec
> was added earlier in this series by "netlink: specs: add genetlink-legacy
> spec for GTP", and its getpdp reply leaves out link:
>
>         reply: &pdp-reply
>           attributes:
>             - version
>             - tid
>             - peer-address
>             - peer-addr6
>             - ms-address
>             - ms-addr6
>             - flow
>             - i-tei
>             - o-tei
>             - family
>
> But gtp_genl_fill_info() always emits it, in every getpdp reply, dump entry
> and notification:
>
>         if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
>             nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
>             nla_put_u8(skb, GTPA_FAMILY, pctx->af))
>
> Should link be added to the reply attributes? Without it, consumers
> generated from the spec can't tell which gtp device a dumped context
> belongs to.
>
> On a related note, this filter matches only on i-tei, not on the link the
> test just created.
>
> > +    ksft_eq(len(pdp), 1)
> > +    ksft_eq(pdp[0]["o-tei"], 0x200)
> > +    ksft_eq(pdp[0]["ms-address"], "10.0.0.1")
> > +    ksft_eq(pdp[0]["peer-address"], "192.0.2.1")
> > +
> > +    gtp.delpdp({"link": link, "version": 1,
> > +                "i-tei": 0x100, "ms-address": "10.0.0.1"})
> > +    ksft_not_in(0x100, _iteis(gtp.getpdp({}, dump=True)))
>
> [ ... ]
>
> > +def del_unknown_pdp(gtp, ns) -> None:
> > +    """Deleting a context that was never added must fail."""
> > +    link = _add_gtp_dev(ns)
> > +    with ksft_raises(NlError):
> > +        gtp.delpdp({"link": link, "version": 1,
> > +                    "i-tei": 0x999, "ms-address": "10.9.9.9"})
>
> [Severity: Low]
> Should this check the errno too? ksft_raises(NlError) accepts any netlink
> error. For example, -EINVAL or -ENODEV from gtp_find_pdp_by_link() or
> gtp_genl_del_pdp() would also pass. The real lookup miss in gtp_find_pdp()
> is -ENOENT.
>
> Also, both delpdp requests in this test send ms-address, and
> gtp_find_pdp_by_link() checks that before the TEI branch:
>
> drivers/net/gtp.c:gtp_find_pdp_by_link() {
>     ...
>         if (nla[GTPA_MS_ADDRESS]) {
>                 __be32 ip = nla_get_be32(nla[GTPA_MS_ADDRESS]);
>                 if (family != AF_INET)
>                         return ERR_PTR(-EINVAL);
>                 return ipv4_pdp_find(gtp, ip);
>     ...
> }
>
> That means the i-tei values passed to delpdp are ignored, and delete by
> TEI through gtp1_pdp_find() is never tested.
>
> With the current kernel this request does return -ENOENT, so the test
> passes for the right reason today. It just checks less than the docstring
> suggests.
>
> > +
> > +
> > +def main() -> None:
> > +    """Run the PDP context tests in a temporary netns."""
> > +    with NetNS() as ns:
> > +        with NetNSEnter(str(ns)):
> > +            gtp = GtpFamily()
> > +        ksft_run([add_del_pdp, add_two_pdp, del_unknown_pdp], args=(gtp, ns))
>
> [Severity: Medium]
> Can GtpFamily() succeed here if gtp.ko isn't loaded yet?
>
> GtpFamily() -> YnlFamily.__init__() -> GenlProtocol('gtp') looks up the
> family id with _genl_load_families(). That sends CTRL_CMD_GETFAMILY with
> NLM_F_DUMP, which the kernel answers from ctrl_dumpfamily(). Only the
> non-dump ctrl_getfamily() path autoloads the module:
>
> net/netlink/genetlink.c:ctrl_getfamily() {
>     ...
>         request_module("net-pf-%d-proto-%d-family-%s",
>                        PF_NETLINK, NETLINK_GENERIC, name);
>     ...
> }
>
> This patch adds CONFIG_GTP=m to the selftest config. On a fresh boot with
> that config, the "gtp" family isn't registered yet, so
> genl_family_name_to_id['gtp'] raises KeyError. YnlFamily.__init__()
> turns that into:
>
>     YnlException(f"Family '{self.yaml['name']}' not supported by the kernel")
>
> That exception isn't a KsftSkipEx and is raised outside ksft_run(). Won't
> the whole script abort with a traceback before any case runs?
>
> gtp.ko would only be autoloaded later, through the rtnl link alias, when
> _add_gtp_dev() runs "ip link add gtp0 type gtp". For the same reason, with
> CONFIG_GTP unset the KsftSkipEx in _add_gtp_dev() is never reached, and the
> test fails instead of skipping.
>
> Would it work to create the gtp device, or otherwise load the module,
> before constructing GtpFamily(), and to treat a missing family as a skip?
>
> > +    ksft_exit()
> > +
> > +
> > +if __name__ == "__main__":
> > +    main()
>
> [ ... ]
>
> > diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py
> > index 08deff756f29b..7edb749710231 100644
> > --- a/tools/testing/selftests/net/lib/py/ynl.py
> > +++ b/tools/testing/selftests/net/lib/py/ynl.py
>
> [ ... ]
>
> > @@ -85,3 +85,9 @@ class PSPFamily(YnlFamily):
> >      def __init__(self, recv_size=0):
> >          super().__init__((SPEC_PATH / Path('psp.yaml')).as_posix(),
> >                           schema='', recv_size=recv_size)
> > +
> > +
> > +class GtpFamily(YnlFamily):
> > +    def __init__(self, recv_size=0):
> > +        super().__init__((SPEC_PATH / Path('gtp.yaml')).as_posix(),
> > +                         schema='', recv_size=recv_size)
>
> [Severity: Medium]
> This wrapper loads the gtp.yaml added earlier in this series by
> "netlink: specs: add genetlink-legacy spec for GTP". Does that spec's
> echoreq request match the driver? The spec lists:
>
>         request:
>           attributes:
>             - link
>             - version
>             - peer-address
>             - peer-addr6
>             - family
>
> gtp_genl_send_echo_req() requires GTPA_MS_ADDRESS, which isn't listed:
>
>         if (!info->attrs[GTPA_VERSION] ||
>             !info->attrs[GTPA_LINK] ||
>             !info->attrs[GTPA_PEER_ADDRESS] ||
>             !info->attrs[GTPA_MS_ADDRESS])
>                 return -EINVAL;
>
> The handler is also IPv4-only and never reads GTPA_PEER_ADDR6 or
> GTPA_FAMILY.
>
> A client generated from the spec would never send ms-address, so its
> echoreq would always fail with -EINVAL.
>
> That spec commit says it is "derived directly from include/uapi/linux/gtp.h
> and the gtp_genl_policy / gtp_genl_ops tables in drivers/net/gtp.c". That
> doesn't seem to hold for echoreq.
>
> Should ms-address be added to the echoreq request, and peer-addr6 and
> family removed?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009060615.4177402-1-anilkaushikwireless%40gmail.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-10 10:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-09  6:06 [PATCH net-next v3 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest Anil Kaushik
2026-10-09  6:06 ` [PATCH net-next v3 1/3] gtp: set a valid genl command in PDP context get and dump replies Anil Kaushik
2026-10-09  6:06 ` [PATCH net-next v3 2/3] netlink: specs: add genetlink-legacy spec for GTP Anil Kaushik
2026-10-10  6:21   ` netdev-bot+sashiko
2026-10-09  6:06 ` [PATCH net-next v3 3/3] selftests: net: add a test for the gtp netlink family Anil Kaushik
2026-10-10  6:21   ` netdev-bot+sashiko
2026-10-10 10:56     ` Anil Kaushik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox