nics: Support PASST user mode backend #2331

mvollmer · 2025-09-23T06:48:12Z

Fixes #2186
Demo: https://youtu.be/L-EWj0dmgoo
Obsolete demo with choice, shows more about port forwarding: https://youtu.be/mhwTZ3CW8iI

Validate inputs, tests
Don't clear old port forwards when new ones will not be accepted by virt-xml. Done by first letting virt-xml override the old elements we want to keep, and then explicitly removing the old ones we don't want to keep.
coverage

Machines: Support for network port forwarding

Cockpit can now configure port forwarding for virtual machines on the user session.

Venefilyn

Checking out the demos. I think the simple port forwarding is fine for now but the complex editing requires a lot more work IMO. It isn't intuitive and the alignment is all over the place.

I think we should experiment with what works for the complex one in a separate PR and keep simple port forwarding here.

Venefilyn · 2025-09-26T11:42:18Z

src/components/vm/nics/nicBody.tsx

+                name: 'passt',
+                desc: 'Userspace PASST stack',
+                detailParagraph: _("Recommended method to provide a virtual LAN with NAT to the outside world. Supports port forwarding.")
+            },
            {
                name: 'user',
-                desc: 'Userspace SLIRP stack',
-                detailParagraph: _("Provides a virtual LAN with NAT to the outside world.")
+                desc: 'Userspace SLIRP stack (legacy)',
+                detailParagraph: _("Legacy method to provide a virtual LAN with NAT to the outside world.")


Why do we display "Userspace SLIRP stack" and such to begin with? Feels a bit weird to me to have it displayed in the select vs just writing "slirp" and "passt". We can in the select dropdown have a description for each entry to make it easier, which would look like this: https://www.patternfly.org/components/menus/menu#with-descriptions

In the latest version, there is no user choice anymore between slirp and passt. Cockpit switches from slirp to passt as required, but will never switch back.

I meant, why do we have "Userspace SLIRP stack" instead of just "slirp"?

I meant, why do we have "Userspace SLIRP stack" instead of just "slirp"?

I don't know. Personally, I would like to avoid saying "slirp" or "passt" in the UI at all. The user should not need to make a decision between the two.

Venefilyn · 2025-09-26T11:43:08Z

src/components/vm/nics/nicBody.tsx

+    for (let i = 0; i < pf.ranges.length; i++) {
+        const r = pf.ranges[i];
+        if (text != "")
+            text += ", ";
+        if (r.exclude == "yes")
+            text += _("excluding ");
+        text += r.start;
+        if (r.end)
+            text += "-" + r.end;
+        if (r.to)
+            text += " → " + r.to + (r.end ? "-" + (Number(r.to) + Number(r.end) - Number(r.start)).toString() : "");
+        if (pf.proto && pf.proto != "tcp")
+            text += "/" + pf.proto;


I really dislike having one-letter variables and would much rather want to keep it expanded a bit

Venefilyn · 2025-09-26T11:44:22Z

src/components/vm/nics/nicBody.tsx

+} : {
+    id: string,
+    item: DialogSimplePortForward,
+    onChange: <K extends keyof DialogSimplePortForward>(idx: number, key: K, value: DialogSimplePortForward[K]) => void,
+    idx: number,
+    removeitem: (idx: number) => void,
+}) => {


Lets split this out to an interface too

interface SimplePortForwardProps { // ... } const SimplePortForward = ({...}): SimplePortForwardProps {...}

mvollmer · 2025-09-29T12:52:18Z

Checking out the demos. I think the simple port forwarding is fine for now but the complex editing requires a lot more work IMO. It isn't intuitive and the alignment is all over the place.

Yes, I didn't try at all to make it nice.

I think we should experiment with what works for the complex one in a separate PR and keep simple port forwarding here.

I think we can postpone that indefinitely until there is clear demand.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

mvollmer · 2025-10-30T08:44:26Z

22 -> 22/tcp even if the port is the same.

change display of the ip addres to 127.0.0.1:22 -> 22/tcp

8080-8090 → 80-90 this does not show it is tcp

Very good suggestions!

cockpituous

There are more than 10 code coverage comments, see the full report here.

jelly · 2025-10-30T11:13:49Z

minor validation issues

@jelly, there was and is no validation what so ever in that dialog, unfortunately. For example, invalid MACs also just cause ERROR XML error: unable to parse mac address '52:54:00:8c:a7:7cvvvvv'

Input validation is of course nice. So should I add it?

Aha, should we make this a follow up then if the whole dialog misses it?

mvollmer · 2025-10-30T11:48:23Z

Ouch, when there is an error adding the new port forwards, we already have cleared out the old ones...

Venefilyn · 2025-10-30T12:46:26Z

change display of the ip addres to 127.0.0.1:22 -> 22/tcp

I agree with this. Looks a lot better and it's following the expected IP:PORT

mvollmer · 2025-10-30T13:51:28Z

minor validation issues

@jelly, there was and is no validation what so ever in that dialog, unfortunately. For example, invalid MACs also just cause ERROR XML error: unable to parse mac address '52:54:00:8c:a7:7cvvvvv'
Input validation is of course nice. So should I add it?

Aha, should we make this a follow up then if the whole dialog misses it?

Nah, I have started on it.

I find this whole topic annoying: We have no clear guidelines and no shareable code and every dialog turns into a spaghetti monster, and does validation slightly differently from any other. [dramatic exaggeration] But I am ready now to do something about this!

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

tomasmatus · 2025-11-06T10:20:42Z

src/components/vm/nics/nic.css

+        /* Move 'Remove' button to the bottom of the line so as to align with the other form fields */
+        display: flex;
+        flex-direction: row;
+        align-items: flex-end;


the button alignment here is not quite right, it's more towrds bottom rather than being on center with the other groups.
You'll need to add the following css to the button element. Same as in podman.

Suggested change

align-items: flex-end;

align-items: flex-end;

+

+ button {

+ margin-block-end: var(--pf-t--global--spacer--xs);

+ }

Before:

After:

Yes, I also noticed that the trashcan is way out of alignment when there is helper text:

I had to align from the top, with a ugly computation:

align-items: flex-start; button { margin-block-start: calc(var(--pf-v6-c-form__label--LineHeight) * var(--pf-v6-c-form__label--FontSize) + var(--pf-v6-c-form__group--Gap) + var(--pf-t--global--spacer--xs)); }

argh, yes. I forgot about the additional padding when helper text shows up. Good catch! And the same problem also needs to be fixed in c-podman's port forwarding.

tomasmatus · 2025-11-06T10:41:59Z

src/components/vm/nics/nicBody.tsx

+}
+
+function validateAddress(addr: string): string | undefined {
+    if (addr && !ipaddr.isValid(addr))


I'd be careful about ipaddr.isValid as it uses ipaddr.IPv4.isValid which also considers "short" variants of IPv4 to be valid. Is libvirt OK with it when it receives IPv4 address that isn't in the typical 4 octets format? AFAIK there is no RFC for such format and you might want to use ipaddr.IPv4.isValidFourPartDecimal instead.

ipaddr.IPv6.isValid which is used in the background should be fine from what I remember from cockpit networking page.

I'd change this to (ipaddr.IPv4.isValidFourPartDecimal(addr) || ipaddr.IPv6.isValid(addr)

after testing it it looks like it's fine or some part of JS code expands the format. Inputting 10.10 into the address field and clicking save changes it to 10.0.0.10.

Ok, thanks for checking!

…uccessful

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicAdd.tsx

        this.state = {
            dialogError: undefined,
-            networkType: "network",
+            networkType: this.props.vm.connectionName == "session" ? "user" : "network",


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicAdd.tsx

        }
+
+        if (this.state.doOnlineValidation)
+            this.setState(state => ({ validation: validateDialogBodyValues(state) }));


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicAdd.tsx

+        if (validation) {
+            this.setState({ validation, doOnlineValidation: true });
+            return;


These 3 added lines are not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicAdd.tsx

                    type: this.state.networkType,
+                    backend: { type: backend },
                    source,
+                    portForward: this.state.networkType == "user" ? dialogPortForwardsToInterface(this.state.portForwards) : null,


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicAdd.tsx

-                                 onValueChanged={this.onValueChanged} />}
-            </Form>
+            <>
+                <Form onSubmit={e => e.preventDefault()} isHorizontal>


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicBody.tsx

+    if (pf.address)
+        text += pf.address + ":";
+    if (pf.dev)
+        text += pf.dev + ":";


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:43Z

src/components/vm/nics/nicBody.tsx

+    if (pf.dev)
+        text += pf.dev + ":";
+    if (!pf.range.some(r => r.exclude != "yes"))
+        text += _("all");


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:44Z

src/components/vm/nics/nicBody.tsx

+        if (r.end)
+            text += "-" + r.end;
+        if (!r.exclude)
+            text += " → " + (r.to || r.start) + (r.end ? "-" + (Number(r.to || r.start) + Number(r.end) - Number(r.start)).toString() : "");


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:44Z

src/components/vm/nics/nicBody.tsx

+            return {
+                kind: "simple",
+                address: pf.address || "",
+                proto: pf.proto || "",


This added line is not executed by any test. Details

cockpituous · 2025-11-06T12:42:44Z

src/components/vm/nics/nicBody.tsx

+                address: pf.address || "",
+                proto: pf.proto || "",
+                host: r.start + (r.end ? "-" + r.end : ""),
+                guest: r.to || "",


This added line is not executed by any test. Details

mvollmer added the no-test label Sep 23, 2025

mvollmer force-pushed the nics-passt branch 2 times, most recently from 95b488b to d6c59ff Compare September 23, 2025 07:44

mvollmer mentioned this pull request Sep 23, 2025

[Feature request] Add Passt and portForward to the create/edit VM page #2186

Open

mvollmer force-pushed the nics-passt branch from d6c59ff to ca3a842 Compare September 25, 2025 09:10

mvollmer requested a review from Venefilyn September 25, 2025 13:08

mvollmer force-pushed the nics-passt branch 2 times, most recently from e361183 to 730cbf8 Compare September 26, 2025 11:30

Venefilyn reviewed Sep 26, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch 3 times, most recently from e5f91a8 to baa4847 Compare September 29, 2025 10:26

mvollmer added the blocked label Sep 29, 2025

mvollmer force-pushed the nics-passt branch from baa4847 to 27627c5 Compare September 29, 2025 11:52

mvollmer force-pushed the nics-passt branch 3 times, most recently from 9f7345f to 548bcc3 Compare September 30, 2025 07:37

mvollmer removed the blocked label Oct 10, 2025

mvollmer force-pushed the nics-passt branch from 548bcc3 to 482f871 Compare October 10, 2025 11:04

mvollmer added the release-note label Oct 13, 2025

mvollmer force-pushed the nics-passt branch from 482f871 to 10e248e Compare October 13, 2025 07:58

mvollmer removed the no-test label Oct 13, 2025

mvollmer force-pushed the nics-passt branch 2 times, most recently from 5ca5232 to 4af3b62 Compare October 13, 2025 09:41

cockpituous reviewed Oct 13, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch 3 times, most recently from bd2aa21 to 57e3277 Compare October 13, 2025 12:38

cockpituous reviewed Oct 13, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch from 3b8315e to a25de69 Compare October 30, 2025 08:46

cockpituous reviewed Oct 30, 2025

View reviewed changes

mvollmer marked this pull request as draft October 30, 2025 11:50

cockpituous reviewed Oct 31, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch from c59dd4a to d117119 Compare October 31, 2025 10:00

cockpituous reviewed Oct 31, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch 2 times, most recently from d9d932d to 86211d6 Compare October 31, 2025 12:21

mvollmer marked this pull request as ready for review October 31, 2025 12:25

mvollmer requested review from Venefilyn and jelly October 31, 2025 12:27

cockpituous reviewed Oct 31, 2025

View reviewed changes

mvollmer force-pushed the nics-passt branch from 86211d6 to 66f0c2f Compare November 4, 2025 07:06

cockpituous reviewed Nov 4, 2025

View reviewed changes

tomasmatus requested changes Nov 6, 2025

View reviewed changes

mvollmer added 6 commits November 6, 2025 13:54

nics: Support user mode port forwarding

a5fa0a9

FIXUP - text representation

dd24cd4

FIXUP - CSS comment style

ad25151

FIXUP - validation

e36ef9c

FIXUP - only remove unwanted portForward elements once virt-xml was s…

5748578

…uccessful

FIXUP - trashcan alignment

02eb967

mvollmer force-pushed the nics-passt branch from 66f0c2f to 02eb967 Compare November 6, 2025 12:22

mvollmer requested a review from tomasmatus November 6, 2025 12:22

cockpituous reviewed Nov 6, 2025

View reviewed changes

nics: Support PASST user mode backend #2331

Are you sure you want to change the base?

nics: Support PASST user mode backend #2331

Uh oh!

Conversation

mvollmer commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Machines: Support for network port forwarding

Uh oh!

Venefilyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvollmer commented Sep 29, 2025

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

mvollmer commented Oct 30, 2025

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

jelly commented Oct 30, 2025

Uh oh!

mvollmer commented Oct 30, 2025

Uh oh!

Venefilyn commented Oct 30, 2025

Uh oh!

mvollmer commented Oct 30, 2025

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvollmer Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cockpituous left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

mvollmer commented Sep 23, 2025 •

edited

Loading

mvollmer Nov 6, 2025 •

edited

Loading