Page MenuHomeFreeBSD

bhyve: move common code to net_utils.c
ClosedPublic

Authored by vmaffione on Jun 12 2019, 8:19 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 25 2024, 9:16 AM
Unknown Object (File)
Feb 15 2024, 7:42 PM
Unknown Object (File)
Jan 3 2024, 8:13 AM
Unknown Object (File)
Jan 1 2024, 12:59 PM
Unknown Object (File)
Dec 30 2023, 3:43 AM
Unknown Object (File)
Nov 9 2023, 10:05 AM
Unknown Object (File)
Nov 1 2023, 8:52 PM
Unknown Object (File)
Oct 3 2023, 4:18 PM

Details

Summary

Both virtio_net and e82545 network frontends have code to validate and generate
MAC addresses. These functionalities are replicated in the two files, so we move
them in a separate compilation unit.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I am pulling Kyle Evans in on this as he just did a bunch of cleanup on mac generation code and there may already be existing code to reused rather than have yet another mac generator.

usr.sbin/bhyve/net_utils.c
3

This I can understand being attributed to NetApp as you took this code from pci_e82545.c.

usr.sbin/bhyve/net_utils.h
3

How is this file attributed to NetApp? I think this is a new file you created, as these functions did not exist before as external.

This revision is now accepted and ready to land.Jun 13 2019, 3:22 PM
This revision now requires review to proceed.Jun 13 2019, 3:29 PM
usr.sbin/bhyve/net_utils.c
82

bhyve already has a set of MAC addresses assigned to it.

/* Organizationally Unique Identifier assigned by IEEE 14 Nov 2013 */
#define OUI_FREEBSD_BASE 0x589cfc000000
#define OUI_FREEBSD(nic) (OUI_FREEBSD_BASE | (nic))
...
/* Allocate 20 bits to bhyve */
#define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD(0x000001)
#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD(0x0fffff)
...

(from net/ieee-oui.h)

this isn't in that range. Is that relevant? Or perhaps a change for another day?

usr.sbin/bhyve/net_utils.h
4

You can omit this phrase (here and elsewhere) if you choose. The project template no longer has it, though it used to.

grehan added inline comments.
usr.sbin/bhyve/net_utils.c
82

The issue with changing it (and why it wasn't done earlier) is that Linux guests would hang on boot when they see a different MAC address, which implied a different interface than the one configured. Maybe things are better these days with systemd.

usr.sbin/bhyve/net_utils.c
82

Makes sense... I'm cool with this just being code motion too...

Removed "All rights reserved".

vmaffione added inline comments.
usr.sbin/bhyve/net_utils.c
82

+1

I am pulling Kyle Evans in on this as he just did a bunch of cleanup on mac generation code and there may already be existing code to reused rather than have yet another mac generator.

We have this facility (in kernel only at the moment), but I think it would ultimately be wrong for bhyve, judging by the description:

followed by an MD5 of the PCI slot/func number and dev name

One could theoretically take advantage of this to have a stable MAC across hosts for cases like, e.g., migration, no? Although it's not clear that bhyve folk would want to maintain this kind of promise, I could see it being somewhat handy.

This revision is now accepted and ready to land.Jun 13 2019, 4:39 PM
This revision was automatically updated to reflect the committed changes.

I am pulling Kyle Evans in on this as he just did a bunch of cleanup on mac generation code and there may already be existing code to reused rather than have yet another mac generator.

...

One could theoretically take advantage of this to have a stable MAC across hosts for cases like, e.g., migration, no? Although it's not clear that bhyve folk would want to maintain this kind of promise, I could see it being somewhat handy.

For the purpose of migration you take the mac with you, that is part of the guest state.

usr.sbin/bhyve/net_utils.c
4

The All rights reserved clause needs to go back here, you can not remove someone else's All rights reserved

usr.sbin/bhyve/net_utils.h
4

The elsewhere statement was missleading here, I think Warner meant for other places you may write original work, you can not omit this when copying someone else's copyrighted work and copyright.

vmaffione added inline comments.
usr.sbin/bhyve/net_utils.c
4

Ok, I'll restore that on the next commit.