[PATCH net-next v4 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest

[PATCH net-next v4 0/3] gtp: ynl spec for the GTP netlink family, a reply-command fix, and a selftest

[Anil Kaushik]

This adds a ynl spec for the gtp generic netlink family, a driver fix the
spec exposed, and 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
PDP contexts with newpdp, checks getpdp dumps them and delpdp removes
them, by TEI and by MS address, and that deleting an unknown context
fails with ENOENT. It skips when the kernel or iproute2 lack GTP
support.

Changes since v3 (thanks to the Sashiko review):
 - spec: echoreq request now matches gtp_genl_send_echo_req(): add
   ms-address (required by the handler) and net-ns-fd (read by
   gtp_find_dev()), drop peer-addr6 and family (never read).
 - spec: delpdp request now lists the attributes the lookup in
   gtp_find_pdp() reads, the same as the getpdp request, instead of
   all the newpdp ones.
 - spec: add link to the getpdp reply, gtp_genl_fill_info() always
   emits it.
 - spec: commit message reworded, the per-command attributes now
   follow the handlers.
 - selftest: create the gtp device, which loads gtp.ko, before the
   family handle: ynl resolves family ids once per process with a dump,
   which does not autoload modules, so with CONFIG_GTP=m the test
   aborted with a traceback. Without GTP support it now skips.
 - selftest: only consider contexts on the test's own gtp device.
 - selftest: delete by TEI only, so the GTPv1 TEI lookup is used, and
   delete by MS address in the two-contexts case.
 - selftest: check that deleting an unknown context fails with ENOENT,
   both by TEI and by MS address.
 - Patch 1 is unchanged from v3.

v3: https://lore.kernel.org/netdev/20261009060615.4177402-1-anilkaushikwireless@gmail.com/
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          | 171 ++++++++++++++++++
 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            | 115 ++++++++++++
 .../testing/selftests/net/lib/py/__init__.py  |   5 +-
 tools/testing/selftests/net/lib/py/ynl.py     |   8 +-
 8 files changed, 302 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/netlink/specs/gtp.yaml
 create mode 100755 tools/testing/selftests/net/gtp.py

-- 
2.25.1

[PATCH net-next v4 1/3] gtp: set a valid genl command in PDP context get and dump replies

[Anil Kaushik]

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

[PATCH net-next v4 2/3] netlink: specs: add genetlink-legacy spec for GTP

[Anil Kaushik]

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. Command and attribute values are taken from
include/uapi/linux/gtp.h and match it one-to-one. The request
attributes of each command are the ones its handler in
drivers/net/gtp.c reads, and the reply attributes the ones
gtp_genl_fill_info() emits.

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 | 171 +++++++++++++++++++++++++++
 MAINTAINERS                          |   1 +
 2 files changed, 172 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..2c61c4353c89
--- /dev/null
+++ b/Documentation/netlink/specs/gtp.yaml
@@ -0,0 +1,171 @@
+# 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-find
+          attributes:
+            - link
+            - version
+            - tid
+            - ms-address
+            - ms-addr6
+            - i-tei
+            - family
+            - net-ns-fd
+    -
+      name: getpdp
+      doc: Get or dump one or more PDP contexts.
+      attribute-set: gtp
+      dont-validate: [strict, dump]
+      flags: [admin-perm]
+      do:
+        request: *pdp-find
+        reply: &pdp-reply
+          attributes:
+            - link
+            - 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
+            - ms-address
+            - net-ns-fd
+
+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

[PATCH net-next v4 3/3] selftests: net: add a test for the gtp netlink family

[Anil Kaushik]

Exercise the gtp generic netlink family through its ynl spec: create a
gtp device, add a PDP context with newpdp, check getpdp dumps it on that
device with the expected TEIDs and addresses, and that delpdp by TEI
removes it. Two more cases check that two contexts are dumped and one
can be deleted by its MS address, and that deleting an unknown context
fails with ENOENT, both by TEI and by MS address.

ynl resolves the family ids once per process, with a CTRL_CMD_GETFAMILY
dump which does not autoload modules, so the gtp device is created
before the family handle: that loads gtp.ko through its rtnl link alias.
The test is skipped when the kernel or iproute2 lack GTP support.

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            | 115 ++++++++++++++++++
 .../testing/selftests/net/lib/py/__init__.py  |   5 +-
 tools/testing/selftests/net/lib/py/ynl.py     |   8 +-
 6 files changed, 128 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..b307699b3c1e
--- /dev/null
+++ b/tools/testing/selftests/net/gtp.py
@@ -0,0 +1,115 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Tests for the gtp netlink family."""
+
+import errno
+
+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 _setup(ns):
+    """Return a gtp family handle, and the ifindex of a new gtp device.
+
+    Create the device first: that loads gtp.ko through its rtnl link
+    alias. ynl resolves the family ids once per process, with a
+    CTRL_CMD_GETFAMILY dump which does not autoload modules, so the
+    family must be registered before the first family handle is created.
+    """
+    try:
+        ip("link add gtp0 type gtp role ggsn", ns=str(ns))
+    except CmdExitFailure as e:
+        raise KsftSkipEx("no gtp support (CONFIG_GTP, iproute2)") from e
+    defer(ip, "link del gtp0", ns=str(ns))
+    ip("link set gtp0 up", ns=str(ns))
+    link = ip("-d link show gtp0", ns=str(ns), json=True)[0]["ifindex"]
+
+    with NetNSEnter(str(ns)):
+        gtp = GtpFamily()
+
+    return gtp, link
+
+
+def _contexts(gtp, link):
+    return [c for c in gtp.getpdp({}, dump=True) if c.get("link") == link]
+
+
+def _iteis(contexts):
+    return [c.get("i-tei") for c in contexts]
+
+
+def add_del_pdp(ns) -> None:
+    """Add a PDP context, query it, then delete it by TEI.
+
+    The dump must return the context with the link, TEIDs and addresses
+    it was created with, and it must be gone after the delete. The delete
+    only identifies the context by its TEI, to use the GTPv1 TEI lookup.
+    """
+    gtp, link = _setup(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 _contexts(gtp, link) 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})
+    ksft_not_in(0x100, _iteis(_contexts(gtp, link)))
+
+
+def add_two_pdp(ns) -> None:
+    """Add two PDP contexts, check both are dumped, delete one by address.
+
+    The delete identifies the context by its MS address, and must leave
+    the other context in place.
+    """
+    gtp, link = _setup(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(_contexts(gtp, link))
+    ksft_in(0x111, iteis)
+    ksft_in(0x112, iteis)
+
+    gtp.delpdp({"link": link, "version": 1, "ms-address": "10.0.1.1"})
+    iteis = _iteis(_contexts(gtp, link))
+    ksft_not_in(0x111, iteis)
+    ksft_in(0x112, iteis)
+
+
+def del_unknown_pdp(ns) -> None:
+    """Deleting a context that was never added must fail with ENOENT.
+
+    Both lookups are checked: by TEI, and by MS address.
+    """
+    gtp, link = _setup(ns)
+
+    for attrs in ({"i-tei": 0x999}, {"ms-address": "10.9.9.9"}):
+        with ksft_raises(NlError) as cm:
+            gtp.delpdp({"link": link, "version": 1, **attrs})
+        ksft_eq(cm.exception.error, errno.ENOENT)
+
+
+def main() -> None:
+    """Run the PDP context tests in a temporary netns."""
+    with NetNS() as ns:
+        ksft_run([add_del_pdp, add_two_pdp, del_unknown_pdp], args=(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

Re: [PATCH net-next v4 2/3] netlink: specs: add genetlink-legacy spec for GTP

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · 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 spec in Documentation/netlink/specs/gtp.yaml lists the
  "gtp" multicast group (lines 168-171), but it never says what the…
- [Medium] Adding gtp.yaml pulls a new compile unit, gtp-user.o, into the
  tools/net/ynl library build.
- [Low] drivers/net/gtp.c registers gtp_genl_family with `.version = 0`
  (line 2483).
- [Low] gtp.yaml declares `checks: exact-len: 16` for peer-addr6 and ms-
  addr6 (lines 71-72 and 79-80).
- [Low] gtp.yaml documents ms-address only as "IPv4 address of the mobile
  subscriber served by the context." (line 48).
- [Low] gtp.yaml documents ms-addr6 as "IPv6 address of the mobile
  subscriber served by the context." (line 83), and its exact-len: 16…

--

netlink: specs: add genetlink-legacy spec for GTP

This adds a genetlink-legacy YAML spec for the existing GTP generic
netlink family. It covers the NEWPDP, DELPDP, GETPDP and ECHOREQ
commands, the GTPA_* attribute set and the "gtp" multicast group. The
spec is also added 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..2c61c4353c89a
> --- /dev/null
> +++ b/Documentation/netlink/specs/gtp.yaml
> @@ -0,0 +1,171 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +---
> +name: gtp
> +
> +protocol: genetlink-legacy

[Severity: Low]
Should the spec state the family's real version? With no version key, the
genetlink-legacy schema default of 1 applies. The kernel registers the
family as version 0:

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

Documentation/netlink/genetlink-legacy.yaml documents "Default is 1" and
sets minimum: 1, so the spec can't say version: 0 unless the schema
changes.

Nothing fails at runtime. ynl hard-codes version 1 in requests and the
genetlink core doesn't check it. Still, the spec gives a different version
from the one the kernel reports in CTRL_ATTR_VERSION and in message
headers.

Could the schema minimum be relaxed so version: 0 can be added? If not,
could the mismatch be documented?

[ ... ]

> +      -
> +        name: ms-address
> +        type: u32
> +        byte-order: big-endian
> +        display-hint: ipv4
> +        doc: IPv4 address of the mobile subscriber served by the context.

[Severity: Low]
Is this description right for echoreq, which also lists ms-address in its
request? gtp_genl_send_echo_req() uses the attribute as the outer IPv4
source address:

	dst_ip = nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
	src_ip = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
	...
	rt = ip4_route_output_gtp(&fl4, sk, dst_ip, src_ip);
	if (IS_ERR(rt)) {
		...
		return -ENODEV;
	}

So the value is the local GSN tunnel endpoint, not a subscriber address.
The echo response notification works the same way: gtp0_handle_echo_resp_ip()
fills ms-address from the local address (iph->daddr).

A user who follows the spec and passes the subscriber's IPv4 address gets
a route lookup with a non-local source, and the command returns -ENODEV.

Could the attribute doc, or the echoreq operation doc, mention this legacy
overloading?

[ ... ]

> +      -
> +        name: peer-addr6
> +        type: binary
> +        checks:
> +          exact-len: 16

[Severity: Low]
Does the kernel actually enforce an exact length here? gtp_genl_policy
has:

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

Neither entry sets .type, so both are NLA_UNSPEC. For NLA_UNSPEC,
validate_nla() only checks attrlen < pt->len, which is a minimum length.
Every gtp op also sets GENL_DONT_VALIDATE_STRICT. As a result the kernel
accepts longer attributes, and nla_get_in6_addr() reads the first 16
bytes.

Should the spec say min-len: 16 instead? The other option is to tighten
the kernel policy to NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)). The
same question applies to ms-addr6 below.

[ ... ]

> +      -
> +        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.

[Severity: Low]
The driver treats ms-addr6 as a /64 prefix whose lower 64 bits must be
zero. ipv6_pdp_fill() rejects NEWPDP otherwise:

	pctx->ms.addr6 = nla_get_in6_addr(info->attrs[GTPA_MS_ADDR6]);
	if (pctx->ms.addr6.s6_addr32[2] ||
	    pctx->ms.addr6.s6_addr32[3])
		return false;

gtp_find_pdp_by_link() applies the same check and returns -EADDRNOTAVAIL
for GETPDP and DELPDP lookups:

		if (addr.s6_addr32[2] ||
		    addr.s6_addr32[3])
			return ERR_PTR(-EADDRNOTAVAIL);

The current text suggests a full subscriber IPv6 address. A context can't
be created, queried or deleted with one.

Could the doc say this attribute carries the subscriber's /64 prefix with
the low 64 bits zeroed?

> +      -
> +        name: family
> +        type: u8
> +        doc: Address family (AF_INET or AF_INET6) of the context addresses.

[Severity: Medium]
Will this spec break the tools/net/ynl build on hosts with older uapi
headers?

tools/net/ynl/generated/Makefile builds every spec except conntrack and
nftables, so gtp-user.c is now generated and compiled. It includes
<linux/gtp.h> and is built with:

	-I../lib/ -idirafter $(UAPI_PATH)

Because of -idirafter, the installed /usr/include/linux/gtp.h takes
precedence over include/uapi/linux/gtp.h. The only way around that is a
CFLAGS_gtp entry in tools/net/ynl/Makefile.deps, and there isn't one.
Later patches in the series don't seem to add one either.

GTPA_PEER_ADDR6, GTPA_MS_ADDR6, GTPA_FAMILY and GTP_CMD_ECHOREQ are
fairly recent. On hosts with 5.x or 6.1-era linux-libc-dev, the generated
code would refer to undefined enumerators. The policy array sized by
GTPA_MAX would also be too small, and make -C tools/net/ynl would fail.

Would adding something like this to Makefile.deps fix it?

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

The guard name assumes headers_install strips _UAPI from
_UAPI_LINUX_GTP_H_.

[ ... ]

> +    -
> +      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
> +            - ms-address
> +            - net-ns-fd
> +
> +mcast-groups:
> +  list:
> +    -
> +      name: gtp

[Severity: Medium]
The gtp multicast group is declared, but which messages go out on it? No
operation has notify, event or mcgrp. The driver multicasts three kinds of
message on GTP_GENL_MCGRP. The first two come from these paths:

gtp_genl_new_pdp()->gtp_tunnel_notify(pctx, GTP_CMD_NEWPDP, GFP_KERNEL)
gtp_genl_del_pdp()->gtp_tunnel_notify(pctx, GTP_CMD_DELPDP, GFP_ATOMIC)

gtp_genl_fill_info() builds both, so they carry the same content as the
getpdp reply.

The third is the echo response. gtp0_handle_echo_resp() and the GTPv1-U
handler both send it:

	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);

It carries version, peer-address and ms-address. The echoreq doit,
gtp_genl_send_echo_req(), only ACKs, so this multicast is the only way
user space learns the echo result.

nlspec only treats ops with notify or event as asynchronous, so
async_msg_ids is empty for gtp. After ntf_subscribe('gtp'), check_ntf() in
ynl.py prints "Unexpected msg id while checking for ntf". It then drops
every PDP add, PDP delete and echo response.

ynl_gen_c doesn't generate notification parsers for these messages
either, and the rendered docs don't mention them. The commit message says
the spec describes "the existing family", but this part of the uAPI is
missing.

Could newpdp and delpdp notifications be described by reusing the getpdp
reply (notify: getpdp, mcgrp: gtp)? The echo response could be an event
with version, peer-address and ms-address that keeps the legacy command
value. Otherwise, could the spec say explicitly that notifications are not
covered?

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

Re: [PATCH net-next v4 3/3] selftests: net: add a test for the gtp netlink family

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In tools/testing/selftests/net/gtp.py, _setup() catches every
  CmdExitFailure from `ip link add gtp0 type gtp role ggsn` and raises…
- [Low] _setup() in tools/testing/selftests/net/gtp.py creates a new
  GtpFamily handle inside the test netns for each test case (`with…

--

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

This adds a Python selftest that uses the gtp generic netlink family
through its ynl spec. It creates a gtp device, then adds, dumps and
deletes PDP contexts by TEI and by MS address. It also checks that
deleting an unknown context returns ENOENT, and it adds a GtpFamily ynl
wrapper.

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

[ ... ]

> +def _setup(ns):
> +    """Return a gtp family handle, and the ifindex of a new gtp device.
> +
> +    Create the device first: that loads gtp.ko through its rtnl link
> +    alias. ynl resolves the family ids once per process, with a
> +    CTRL_CMD_GETFAMILY dump which does not autoload modules, so the
> +    family must be registered before the first family handle is created.
> +    """
> +    try:
> +        ip("link add gtp0 type gtp role ggsn", ns=str(ns))
> +    except CmdExitFailure as e:
> +        raise KsftSkipEx("no gtp support (CONFIG_GTP, iproute2)") from e

[Severity: Low]
Is this skip broader than intended? CmdExitFailure is raised for any
non-zero exit from ip. That covers old iproute2 and a missing gtp.ko,
but it also covers a real failure in the gtp newlink path. Examples are
gtp_newlink() failing to create or bind its UDP sockets, an -EINVAL
regression, or -EEXIST if an earlier case left a gtp0 behind.

add_del_pdp(), add_two_pdp() and del_unknown_pdp() all go through
_setup(). If gtp link creation regresses, every case would be reported
as SKIP instead of FAIL.

The patch also adds CONFIG_GTP=m to tools/testing/selftests/net/config,
so a correctly configured run should not need this skip. Would it make
sense to check e.cmd.stderr for the specific "not supported" messages
and re-raise anything else? drivers/net/lib/py/env.py already uses that
pattern:

            except CmdExitFailure as e:
                if "Operation not supported" not in e.cmd.stderr:
                    raise

> +    defer(ip, "link del gtp0", ns=str(ns))
> +    ip("link set gtp0 up", ns=str(ns))
> +    link = ip("-d link show gtp0", ns=str(ns), json=True)[0]["ifindex"]
> +
> +    with NetNSEnter(str(ns)):
> +        gtp = GtpFamily()
> +
> +    return gtp, link

[Severity: Low]
This isn't a functional bug, but should the GtpFamily handle be closed
when each case ends?

YnlFamily.__init__() opens a netlink socket. It also stores a
functools.partial(self._op, op_name) on self for every op, which makes a
reference cycle:

        for op_name, op in self.ops.items():
            bound_f = functools.partial(self._op, op_name)
            setattr(self, op.ident_name, bound_f)

Because of that cycle, reference counting alone never frees the socket.
It is freed only by the cyclic GC or at process exit. Each case creates
a new handle, so up to three netlink sockets in the test netns stay
open. Each one keeps the namespace alive after NetNS.__exit__() deletes
it.

YnlFamily already has close() and __enter__()/__exit__(). Would adding
defer(gtp.close) in _setup() be cleaner?

[ ... ]

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