svn commit: r349019 - head/usr.sbin/bhyve

Rodney W. Grimes freebsd at gndrsh.dnsmgr.net
Thu Jun 13 17:46:39 UTC 2019


> 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 <md5.h>
> +#include <net/ethernet.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +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 <v.maffione at gmail.com>

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 <stdint.h>
> +#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 at freebsd.org


More information about the svn-src-head mailing list