Skip to content

hvsock related systemd and nm config files#202

Merged
cfergeau merged 1 commit intocontainers:mainfrom
baude:podmanstuff
Apr 21, 2023
Merged

hvsock related systemd and nm config files#202
cfergeau merged 1 commit intocontainers:mainfrom
baude:podmanstuff

Conversation

@baude
Copy link
Copy Markdown
Member

@baude baude commented Apr 14, 2023

the network manager file configures a vsock0 network interface with the specific mac-address that gvproxy needs. this must be run before the vm command is run.

the systemd file is for running the vm command after network manager creates the network interface. the url must be modified with the vm ID when using hvsock and Microsoft HyperV virtualization. Once the vm command has been run, we issue a postexec to ask NM to bring the "link" up. this may not be entirely necessary but wfm.

Copy link
Copy Markdown
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these files, these were the missing bits after the addition of --preexisting!

@@ -0,0 +1,10 @@
[Unit]
Description=vsock_network
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use a more descriptive name, such as gvisor-tap-vsock traffic forwarder, or Network Traffic Forwarder over virtio-vsock

@@ -0,0 +1,10 @@
[Unit]
Description=vsock_network
After=NetworkManager.service
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add After=sys-devices-virtual-net-vsock0.device, and possibly BindsTo=sys-devices-virtual-net-vsock0.device, but I think with the latter Praveen had issues with systemctl restart user_network.service

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i know this works as is ... do you want to change it when you do a switch over?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll experiment a bit with this and the removal of ExecStartPost=/usr/bin/nmcli c up vsock0 and let you know!

@cfergeau
Copy link
Copy Markdown
Collaborator

Fwiw, the golangci-lint failures are fixed in git main

@baude
Copy link
Copy Markdown
Member Author

baude commented Apr 19, 2023

updated

Copy link
Copy Markdown
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 small things to fix, apart from this looks good to me!

After=NetworkManager.service

[Service]
[Service]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate Service

Environment=GV_TAP_DEVICE="vsock0"
Environment=GV_VSOCK_PORT="1234"
EnvironmentFile=-/etc/sysconfig/gv-user-network
ExecStart=/usr/libexec/podman/vm -preexisting -iface $GV_TAP_DEVICE -url vsock://2:$GV_VSOCK_PORT/connect
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, systemd wants {} around the env var names, otherwise it won't expand GV_VSOCK_PORT: ExecStart=/usr/libexec/podman/vm -preexisting -iface ${GV_TAP_DEVICE} -url vsock://2:${GV_VSOCK_PORT}/connect

the network manager file configures a vsock0 network interface with the
specific mac-address that gvproxy needs.  this must be run before the vm
command is run.

the systemd file is for running the vm command after network manager
creates the network interface.  the url must be modified with the vm ID
when using hvsock and Microsoft HyperV virtualization.  Once the vm
command has been run, we issue a postexec to ask NM to bring the "link"
up.  this may not be entirely necessary but wfm.

Signed-off-by: Brent Baude <bbaude@redhat.com>
Copy link
Copy Markdown
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the 2 issues I last mentioned, and changed the default port from 1234 to 1024 since this is what vm uses by default.

@cfergeau
Copy link
Copy Markdown
Collaborator

/lgtm
/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 21, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: baude, cfergeau

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cfergeau cfergeau merged commit 7d1dd1c into containers:main Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants