Page MenuHomeFreeBSD

vmrun: Default to "vmnet"-style tap devices
AbandonedPublic

Authored by cem on Dec 15 2018, 6:42 PM.

Details

Reviewers
glebius
avg
allanjude
rgrimes
araujo
Group Reviewers
bhyve
Summary

These still don't auto-UP themselves on creation, unfortunately, but they remain up once so-configured, so rebooting a VM works.

Suggested by: glebius

Test Plan
$ procstat -f $(pidof bhyve)
  PID COMM                FD T V FLAGS    REF  OFFSET PRO NAME
87493 bhyve             text v r r-------   -       - -   /usr/sbin/bhyve
...
87493 bhyve                4 v c rw---n--   2   23792 -   /dev/vmnet0
...

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 21593
Build 20896: arc lint + arc unit

Event Timeline

cem created this revision.Dec 15 2018, 6:42 PM
cem retitled this revision from vmrun: Default to "vmnet" (online) tap devices to vmrun: Default to "vmnet"-style tap devices.Dec 15 2018, 8:00 PM
cem edited the summary of this revision. (Show Details)
cem edited the test plan for this revision. (Show Details)

I do not like changing defaults without very good reasons, what reasons are there for making this change?

cem added a comment.Dec 15 2018, 8:49 PM

I do not like changing defaults without very good reasons, what reasons are there for making this change?

If you have not yet done so, please read the summary included in the review. If you have read it and still have this question, it could probably stand to be spelled out more clearly.

In D18574#395968, @cem wrote:

I do not like changing defaults without very good reasons, what reasons are there for making this change?

If you have not yet done so, please read the summary included in the review. If you have read it and still have this question, it could probably stand to be spelled out more clearly.

I have read it, I also run many vm's in bhyve, I use tap devices, I reboot my vm's now and then and have no issues. So AGAIN what reasons are there for making these changes and spell it out.

In D18574#395968, @cem wrote:

I do not like changing defaults without very good reasons, what reasons are there for making this change?

If you have not yet done so, please read the summary included in the review. If you have read it and still have this question, it could probably stand to be spelled out more clearly.

I have read it, I also run many vm's in bhyve, I use tap devices, I reboot my vm's now and then and have no issues. So AGAIN what reasons are there for making these changes and spell it out.

tap devices auto-up themselves when the net.link.tap.up_on_open sysctl node is set to 1. I agree that this patch isn't really needed and would represent a POLA violation for those who already rely on vmrun.sh and its default use of tap0. One workaround to the POLA violation would be to explicitly define the device to use via command-line arguments, which is what I do.

araujo requested changes to this revision.Dec 16 2018, 3:06 AM
araujo added a subscriber: araujo.

Hi guys,

Lets discuss a bit more about it, I personally have one concern related with vmnet device regarding when we shutdown a guest VM and the interface will still be up, for VM reboot it seems to be a solution but for a guest shutdown I'm not sure if keeping the vmnet up will gives us any issues. Maybe I'm overseeing it.

One possible solution would be to give the options for users to choose between tap and vmnet style to be used and also document it (we can add some info on our bhyve wiki page)!

Any additional thought about that are welcome!!!

This revision now requires changes to proceed.Dec 16 2018, 3:06 AM

Hi guys,

...

One possible solution would be to give the options for users to choose between tap and vmnet style to be used and also document it (we can add some info on our bhyve wiki page)!

There already is a documented -t option that allows one to change this setting on the command line, it is in the usage() message. vmrun.sh is an example script and needs to die sooner, rather than later, with less effort spent on it and more effort on a new config file syntax that gets us out of using this "temporary" script.

Hi guys,

...

One possible solution would be to give the options for users to choose between tap and vmnet style to be used and also document it (we can add some info on our bhyve wiki page)!

There already is a documented -t option that allows one to change this setting on the command line, it is in the usage() message. vmrun.sh is an example script and needs to die sooner, rather than later, with less effort spent on it and more effort on a new config file syntax that gets us out of using this "temporary" script.

I agree with it, and this was discussed for years already! So, IMHO just close the review, "thanks for that" and we move forward!!!

cem abandoned this revision.Dec 20 2018, 7:44 PM

Obviously, I was already aware of the -t option, and I was also aware of net.link.tap.up_on_open. I understand there are ways to work around this without changing the default; that's not really the point.

The point is it is useful to have sane defaults that provide a good experience without excessive options. (It seems to me that if a user requests a VM run with a network device, the network device should function regularly inside the VM without additional coordination in the hypervisor.)

Given the resistance to this, I guess I will fork vmrun.sh in a personal repository instead and steer clear of bhyve work in the future.

vmrun.sh is an example script and needs to die sooner, rather than later

It seems like you're simultaneously of the opinion that this script must remain totally unmodified, and at the same time suggesting removing it entirely; these views seem mutually incompatible to me.