From owner-svn-src-all@freebsd.org Thu Jun 13 17:46:39 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8000315B8C40; Thu, 13 Jun 2019 17:46:39 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id CF6EE8EFF0; Thu, 13 Jun 2019 17:46:38 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id x5DHkUsY093160; Thu, 13 Jun 2019 10:46:30 -0700 (PDT) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id x5DHkUgr093159; Thu, 13 Jun 2019 10:46:30 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201906131746.x5DHkUgr093159@gndrsh.dnsmgr.net> Subject: Re: svn commit: r349019 - head/usr.sbin/bhyve In-Reply-To: <201906131739.x5DHdX93034129@repo.freebsd.org> To: Vincenzo Maffione Date: Thu, 13 Jun 2019 10:46:30 -0700 (PDT) CC: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: CF6EE8EFF0 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.95 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.95)[-0.946,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Jun 2019 17:46:39 -0000 > Author: vmaffione > Date: Thu Jun 13 17:39:32 2019 > New Revision: 349019 > URL: https://svnweb.freebsd.org/changeset/base/349019 > > Log: > bhyve: move common code to net_utils.c > > 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. > > Reviewed by: rgrimes, bryanv, imp, kevans > MFC after: 2 weeks > Differential Revision: https://reviews.freebsd.org/D20626 You should of waited for a bit longer for additional comments based on the fact you made changes based on other comments. > Added: > head/usr.sbin/bhyve/net_utils.c (contents, props changed) > head/usr.sbin/bhyve/net_utils.h (contents, props changed) > Modified: > head/usr.sbin/bhyve/Makefile > head/usr.sbin/bhyve/pci_e82545.c > head/usr.sbin/bhyve/pci_virtio_net.c > > Modified: head/usr.sbin/bhyve/Makefile > ============================================================================== > --- head/usr.sbin/bhyve/Makefile Thu Jun 13 16:34:55 2019 (r349018) > +++ head/usr.sbin/bhyve/Makefile Thu Jun 13 17:39:32 2019 (r349019) > @@ -32,6 +32,7 @@ SRCS= \ > mem.c \ > mevent.c \ > mptbl.c \ > + net_utils.c \ > pci_ahci.c \ > pci_e82545.c \ > pci_emul.c \ > > Added: head/usr.sbin/bhyve/net_utils.c > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/usr.sbin/bhyve/net_utils.c Thu Jun 13 17:39:32 2019 (r349019) > @@ -0,0 +1,83 @@ > +/*- > + * Copyright (c) 2011 NetApp, Inc. The all rights reserved clause must be restored here, please see the review for the reason. Thanks, Rod > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS > + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, > + * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT > + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, > + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE > + * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, > + * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + * > + * $FreeBSD$ > + */ > + > +#include "net_utils.h" > +#include "bhyverun.h" > +#include > +#include > +#include > +#include > +#include > + > +int > +net_parsemac(char *mac_str, uint8_t *mac_addr) > +{ > + struct ether_addr *ea; > + char *tmpstr; > + char zero_addr[ETHER_ADDR_LEN] = { 0, 0, 0, 0, 0, 0 }; > + > + tmpstr = strsep(&mac_str,"="); > + > + if ((mac_str != NULL) && (!strcmp(tmpstr,"mac"))) { > + ea = ether_aton(mac_str); > + > + if (ea == NULL || ETHER_IS_MULTICAST(ea->octet) || > + memcmp(ea->octet, zero_addr, ETHER_ADDR_LEN) == 0) { > + fprintf(stderr, "Invalid MAC %s\n", mac_str); > + return (EINVAL); > + } else > + memcpy(mac_addr, ea->octet, ETHER_ADDR_LEN); > + } > + > + return (0); > +} > + > +void > +net_genmac(struct pci_devinst *pi, uint8_t *macaddr) > +{ > + /* > + * The default MAC address is the standard NetApp OUI of 00-a0-98, > + * followed by an MD5 of the PCI slot/func number and dev name > + */ > + MD5_CTX mdctx; > + unsigned char digest[16]; > + char nstr[80]; > + > + snprintf(nstr, sizeof(nstr), "%d-%d-%s", pi->pi_slot, > + pi->pi_func, vmname); > + > + MD5Init(&mdctx); > + MD5Update(&mdctx, nstr, (unsigned int)strlen(nstr)); > + MD5Final(digest, &mdctx); > + > + macaddr[0] = 0x00; > + macaddr[1] = 0xa0; > + macaddr[2] = 0x98; > + macaddr[3] = digest[0]; > + macaddr[4] = digest[1]; > + macaddr[5] = digest[2]; > +} > > Added: head/usr.sbin/bhyve/net_utils.h > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/usr.sbin/bhyve/net_utils.h Thu Jun 13 17:39:32 2019 (r349019) > @@ -0,0 +1,37 @@ > +/*- > + * Copyright (c) 2019 Vincenzo Maffione This one is fine, thank you for fixing this. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS > + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, > + * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT > + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, > + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE > + * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, > + * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + * > + * $FreeBSD$ > + */ > + > +#ifndef _NET_UTILS_H_ > +#define _NET_UTILS_H_ > + > +#include > +#include "pci_emul.h" > + > +void net_genmac(struct pci_devinst *pi, uint8_t *macaddr); > +int net_parsemac(char *mac_str, uint8_t *mac_addr); > + > +#endif /* _NET_UTILS_H_ */ > > Modified: head/usr.sbin/bhyve/pci_e82545.c > ============================================================================== > --- head/usr.sbin/bhyve/pci_e82545.c Thu Jun 13 16:34:55 2019 (r349018) > +++ head/usr.sbin/bhyve/pci_e82545.c Thu Jun 13 17:39:32 2019 (r349019) > @@ -65,6 +65,7 @@ __FBSDID("$FreeBSD$"); > #include "bhyverun.h" > #include "pci_emul.h" > #include "mevent.h" > +#include "net_utils.h" > > /* Hardware/register definitions XXX: move some to common code. */ > #define E82545_VENDOR_ID_INTEL 0x8086 > @@ -2259,38 +2260,16 @@ e82545_open_tap(struct e82545_softc *sc, char *opts) > } > > static int > -e82545_parsemac(char *mac_str, uint8_t *mac_addr) > -{ > - struct ether_addr *ea; > - char *tmpstr; > - char zero_addr[ETHER_ADDR_LEN] = { 0, 0, 0, 0, 0, 0 }; > - > - tmpstr = strsep(&mac_str,"="); > - if ((mac_str != NULL) && (!strcmp(tmpstr,"mac"))) { > - ea = ether_aton(mac_str); > - if (ea == NULL || ETHER_IS_MULTICAST(ea->octet) || > - memcmp(ea->octet, zero_addr, ETHER_ADDR_LEN) == 0) { > - fprintf(stderr, "Invalid MAC %s\n", mac_str); > - return (1); > - } else > - memcpy(mac_addr, ea->octet, ETHER_ADDR_LEN); > - } > - return (0); > -} > - > -static int > e82545_init(struct vmctx *ctx, struct pci_devinst *pi, char *opts) > { > - DPRINTF("Loading with options: %s\r\n", opts); > - > - MD5_CTX mdctx; > - unsigned char digest[16]; > char nstr[80]; > struct e82545_softc *sc; > char *devname; > char *vtopts; > int mac_provided; > > + DPRINTF("Loading with options: %s\r\n", opts); > + > /* Setup our softc */ > sc = calloc(1, sizeof(*sc)); > > @@ -2340,7 +2319,7 @@ e82545_init(struct vmctx *ctx, struct pci_devinst *pi, > (void) strsep(&vtopts, ","); > > if (vtopts != NULL) { > - err = e82545_parsemac(vtopts, sc->esc_mac.octet); > + err = net_parsemac(vtopts, sc->esc_mac.octet); > if (err != 0) { > free(devname); > return (err); > @@ -2355,24 +2334,8 @@ e82545_init(struct vmctx *ctx, struct pci_devinst *pi, > free(devname); > } > > - /* > - * The default MAC address is the standard NetApp OUI of 00-a0-98, > - * followed by an MD5 of the PCI slot/func number and dev name > - */ > if (!mac_provided) { > - snprintf(nstr, sizeof(nstr), "%d-%d-%s", pi->pi_slot, > - pi->pi_func, vmname); > - > - MD5Init(&mdctx); > - MD5Update(&mdctx, nstr, strlen(nstr)); > - MD5Final(digest, &mdctx); > - > - sc->esc_mac.octet[0] = 0x00; > - sc->esc_mac.octet[1] = 0xa0; > - sc->esc_mac.octet[2] = 0x98; > - sc->esc_mac.octet[3] = digest[0]; > - sc->esc_mac.octet[4] = digest[1]; > - sc->esc_mac.octet[5] = digest[2]; > + net_genmac(pi, sc->esc_mac.octet); > } > > /* H/w initiated reset */ > > Modified: head/usr.sbin/bhyve/pci_virtio_net.c > ============================================================================== > --- head/usr.sbin/bhyve/pci_virtio_net.c Thu Jun 13 16:34:55 2019 (r349018) > +++ head/usr.sbin/bhyve/pci_virtio_net.c Thu Jun 13 17:39:32 2019 (r349019) > @@ -67,6 +67,7 @@ __FBSDID("$FreeBSD$"); > #include "pci_emul.h" > #include "mevent.h" > #include "virtio.h" > +#include "net_utils.h" > > #define VTNET_RINGSZ 1024 > > @@ -698,29 +699,6 @@ pci_vtnet_ping_ctlq(void *vsc, struct vqueue_info *vq) > } > #endif > > -static int > -pci_vtnet_parsemac(char *mac_str, uint8_t *mac_addr) > -{ > - struct ether_addr *ea; > - char *tmpstr; > - char zero_addr[ETHER_ADDR_LEN] = { 0, 0, 0, 0, 0, 0 }; > - > - tmpstr = strsep(&mac_str,"="); > - > - if ((mac_str != NULL) && (!strcmp(tmpstr,"mac"))) { > - ea = ether_aton(mac_str); > - > - if (ea == NULL || ETHER_IS_MULTICAST(ea->octet) || > - memcmp(ea->octet, zero_addr, ETHER_ADDR_LEN) == 0) { > - fprintf(stderr, "Invalid MAC %s\n", mac_str); > - return (EINVAL); > - } else > - memcpy(mac_addr, ea->octet, ETHER_ADDR_LEN); > - } > - > - return (0); > -} > - > static void > pci_vtnet_tap_setup(struct pci_vtnet_softc *sc, char *devname) > { > @@ -795,9 +773,6 @@ pci_vtnet_netmap_setup(struct pci_vtnet_softc *sc, cha > static int > pci_vtnet_init(struct vmctx *ctx, struct pci_devinst *pi, char *opts) > { > - MD5_CTX mdctx; > - unsigned char digest[16]; > - char nstr[80]; > char tname[MAXCOMLEN + 1]; > struct pci_vtnet_softc *sc; > char *devname; > @@ -834,7 +809,7 @@ pci_vtnet_init(struct vmctx *ctx, struct pci_devinst * > (void) strsep(&vtopts, ","); > > if (vtopts != NULL) { > - err = pci_vtnet_parsemac(vtopts, sc->vsc_config.mac); > + err = net_parsemac(vtopts, sc->vsc_config.mac); > if (err != 0) { > free(devname); > return (err); > @@ -851,24 +826,8 @@ pci_vtnet_init(struct vmctx *ctx, struct pci_devinst * > free(devname); > } > > - /* > - * The default MAC address is the standard NetApp OUI of 00-a0-98, > - * followed by an MD5 of the PCI slot/func number and dev name > - */ > if (!mac_provided) { > - snprintf(nstr, sizeof(nstr), "%d-%d-%s", pi->pi_slot, > - pi->pi_func, vmname); > - > - MD5Init(&mdctx); > - MD5Update(&mdctx, nstr, strlen(nstr)); > - MD5Final(digest, &mdctx); > - > - sc->vsc_config.mac[0] = 0x00; > - sc->vsc_config.mac[1] = 0xa0; > - sc->vsc_config.mac[2] = 0x98; > - sc->vsc_config.mac[3] = digest[0]; > - sc->vsc_config.mac[4] = digest[1]; > - sc->vsc_config.mac[5] = digest[2]; > + net_genmac(pi, sc->vsc_config.mac); > } > > /* initialize config space */ > > -- Rod Grimes rgrimes@freebsd.org