On Thu, 5 Nov 2026 11:30:36 -0800
Bobby Eshleman <[email protected]> wrote:

> The guest's vsock device is not a netdev, so rtnetlink cannot move it
> between network namespaces. Add an "ip vsock" object that goes through
> the "vsock" generic netlink family instead:
> 
>   ip vsock set netns { PID | NETNSNAME | NETNSFILE }
>   ip vsock show
> 
> "set" issues VSOCK_CMD_DEV_NETNS_SET from inside the target namespace,
> since the kernel moves the device to the namespace the request was sent
> from. "show" issues VSOCK_CMD_DEV_NETNS_GET and prints where the device
> is, relative to the current namespace.
> 
> Link: 
> https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Bobby Eshleman <[email protected]>
> ---

Semi-automated review using review-prompts for iproute2


On Thu, 5 Nov 2026 11:30:36 -0800, Bobby Eshleman wrote:

Thanks, this is mostly in good shape. A few things to fix before it
goes beyond RFC.

> +#include <linux/vsock_netlink.h>

This depends on a uapi header (and kernel series) that is not yet
merged. iproute2-next can only take this once the kernel side is in
net-next; the header update should then come from the normal uapi
sync rather than a hand-added copy in patch 1/2.

> +static int vsock_talk(__u8 cmd, struct nlmsghdr **answer)
> +{
> +     GENL_REQUEST(req, 128, 0, 0, VSOCK_FAMILY_VERSION, cmd, NLM_F_REQUEST);
> +     struct rtnl_handle grth = { .fd = -1 };
> +     int family = -1;
> +     int ret;
> +
> +     if (genl_init_handle(&grth, VSOCK_FAMILY_NAME, &family)) {
> +             fprintf(stderr,
> +                     "Cannot resolve the \"%s\" netlink family, the kernel 
> may not support moving the vsock device.\n",
> +                     VSOCK_FAMILY_NAME);
> +             return -1;
> +     }

genl_init_handle() leaves the socket open when genl_resolve_family()
fails, so this leaks grth.fd. Harmless today since ip exits, but not
under "ip -batch". Add rtnl_close(&grth) on this path.

Also genl_resolve_family() already prints "Error talking to the kernel"
on failure, so the user gets two messages. Not a big deal, just be aware.

> +static void vsock_print_netnsid(int id)
> +{
> +     char *name;
> +
> +     if (is_json_context()) {
> +             print_int(PRINT_JSON, "netnsid", NULL, id);
> +             return;
> +     }

This copies the IFLA_LINK_NETNSID pattern from ipaddress.c, which is
fine for consistency, but note that JSON users never get the netns
name and get -1 for "unknown". Consider also emitting
print_string(PRINT_JSON, "netns", NULL, name) when a name exists.

> +int do_ipvsock(int argc, char **argv)
> +{
> +     if (argc < 1)
> +             return vsock_dev_netns_show();
> +
> +     if (matches(*argv, "show") == 0 ||
> +         matches(*argv, "list") == 0) {
...
> +     if (matches(*argv, "set") == 0)
...
> +     if (matches(*argv, "help") == 0)
> +             usage();

and in vsock_set():

> +             } else if (matches(*argv, "help") == 0) {

New code must use strcmp(), not matches(). Abbreviation matching has
caused repeated breakage when keywords are added later (e.g. "s" here
already matches both "show" and "set"; order decides which wins).
Please convert all of these to strcmp().

> +     ret = rtnl_talk(&grth, &req.n, answer);
> +     rtnl_close(&grth);
> +
> +     return ret < 0 ? -2 : 0;

The -2 propagates out as the exit status of ip (254). Other ip
subcommands return -1/1 on failure; either is fine but please be
consistent with vsock_dev_netns_set() which returns -1 for its own
errors and -2 for a kernel error.

> +             "Usage: ip vsock set netns { PID | NETNSNAME | NETNSFILE }\n"
> +             "       ip vsock show\n");

Literal tabs in the usage string; the rest of ip uses spaces for
alignment here ("Usage: ip vsock ...\n       ip vsock show\n").
Also "ip vsock help" should be listed.

> +static int vsock_dev_netns_set(const char *netns)
> +{
...
> +     if (setns(netns_fd, CLONE_NEWNET) < 0) {

Design note, more for the kernel side than this patch: because
VSOCK_CMD_DEV_NETNS_SET has no target attribute, userspace has to do a
setns() dance, and "ip -n foo vsock set netns bar" ends up depending on
which namespace the genl socket was opened in. rtnetlink and devlink
both take an explicit NETNS_FD / NETNS_PID / NETNS_ID attribute for
this; doing the same for vsock would make the tool trivial and avoid
needing setns() (and the privileges to enter the target ns) at all.
Since this is RFC on both sides it seems worth reconsidering now.

If the kernel API stays as is, the setns/restore logic looks correct.

> +ip vsock set netns 1
> +.RS 4
> +Move the device back to the initial network namespace, undoing the 
> assignment.

That only holds if ip is run in the initial pid namespace; "1" is a
PID, not "the initial netns". Maybe say so, or use /proc/1/ns/net.

Minor: the man page says "Nothing follows vsock when the device is in
the current namespace" -- worth also showing an example of the
"netnsid unknown" output so users know what it means.

Reply via email to