svn commit: r231987 - in head/sys/mips/cavium: . octe
Juli Mallett
jmallett at FreeBSD.org
Fri Feb 24 08:39:00 UTC 2012
On Tue, Feb 21, 2012 at 17:30, Oleksandr Tymoshenko <gonzo at freebsd.org> wrote:
> Author: gonzo
> Date: Wed Feb 22 01:30:25 2012
> New Revision: 231987
> URL: http://svn.freebsd.org/changeset/base/231987
>
> Log:
> Refctor address assignment for Octeon's ethernet ports:
>
> - Centralize address assignment
> - Make sure managment ports get first MAC address in pool
It was the case that this would happen, now we're relying on object
link order to do the right thing, which I think is a mistake. This is
a fix for a bug introduced by this patch.
I don't like the idea of handing out MAC addresses to ports as they're
configured, as this *does* break things if you don't have the octm
driver in your kernel, but do have management ports.
Your bootloader comes up, uses the same logic we were using to assign
MACs (which is the gold standard in publicly-available Octeonland,
even if there are private bootloaders and kernels which did
differently), and then when the kernel boots, the MAC assigned to the
physical ports is different. If you use DHCP + bootp on the
management port to boot up, and then configure octe0 via DHCP, your
lease moves from one physical port to another. In my experience,
before I fixed things to match bootloader behavior, this could result
in mysterious and exciting behavior involving long delays in
connectivity at boot.
And of course, now you'll get a different MAC for octe0 based on
whether octm is in your kernel or not. Which is its own set of fun.
The Octeon hardware doesn't need a per-port MAC you can read because
there is a consistent way to allocate them to ports that is easy to
follow. I don't see the benefit in handing them out dynamically,
especially relying in link order to get it right. The previous method
had shortcomings and a load of comments since there were dragons in
the vicinity. This introduces a lot more magic, actually-confusing
behavior, etc., and has few comments, and none that catch eyes in the
right places.
If we want to abstract this out into cvm_assign_mac_address so that
downstream consumers of this code (or upstream, as the case may be)
can do their own thing, I'd suggest passing down the interface number,
whether it's a management port, etc., and restoring the original,
consistent logic, in that function. This has the potential to be
markedly-astonishing. What happens if we add support for loading octe
and octm as modules (which has been requested by consumers of the
Octeon code in the past)? Yikes!
> - Properly propagate fail if address allocation failed
>
> Submitted by: Andrew Duane <aduane at juniper.net>
>
> Modified:
> head/sys/mips/cavium/files.octeon1
> head/sys/mips/cavium/if_octm.c
> head/sys/mips/cavium/octe/ethernet-common.c
> head/sys/mips/cavium/octe/ethernet-common.h
> head/sys/mips/cavium/octe/ethernet-rgmii.c
> head/sys/mips/cavium/octe/ethernet-sgmii.c
> head/sys/mips/cavium/octe/ethernet-spi.c
> head/sys/mips/cavium/octe/ethernet-xaui.c
> head/sys/mips/cavium/octe/ethernet.c
>
> Modified: head/sys/mips/cavium/files.octeon1
> ==============================================================================
> --- head/sys/mips/cavium/files.octeon1 Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/files.octeon1 Wed Feb 22 01:30:25 2012 (r231987)
> @@ -24,6 +24,10 @@ mips/cavium/cryptocteon/cryptocteon.c o
> mips/mips/octeon_cop2_swtch.S standard
> mips/mips/octeon_cop2.c standard
>
> +# octm must be first, so management ports get the first MAC addresses
> +mips/cavium/if_octm.c optional octm
> +contrib/octeon-sdk/cvmx-mgmt-port.c optional octm
> +
> mips/cavium/octe/ethernet.c optional octe
> mips/cavium/octe/ethernet-mv88e61xx.c optional octe octeon_vendor_lanner
> mips/cavium/octe/ethernet-common.c optional octe
> @@ -39,10 +43,6 @@ mips/cavium/octe/mv88e61xxphy.c option
> mips/cavium/octe/octe.c optional octe
> mips/cavium/octe/octebus.c optional octe
>
> -mips/cavium/if_octm.c optional octm
> -
> -contrib/octeon-sdk/cvmx-mgmt-port.c optional octm
> -
> mips/cavium/octopci.c optional pci
> mips/cavium/octopci_bus_space.c optional pci
>
>
> Modified: head/sys/mips/cavium/if_octm.c
> ==============================================================================
> --- head/sys/mips/cavium/if_octm.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/if_octm.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -63,6 +63,7 @@
> #include <contrib/octeon-sdk/cvmx.h>
> #include <contrib/octeon-sdk/cvmx-interrupt.h>
> #include <contrib/octeon-sdk/cvmx-mgmt-port.h>
> +#include "octe/ethernet-common.h"
>
> struct octm_softc {
> struct ifnet *sc_ifp;
> @@ -176,9 +177,10 @@ octm_attach(device_t dev)
> /*
> * Set MAC address for this management port.
> */
> - mac = 0;
> - memcpy((u_int8_t *)&mac + 2, cvmx_sysinfo_get()->mac_addr_base, 6);
> - mac += sc->sc_port;
> + if (cvm_assign_mac_address(&mac, NULL) != 0) {
> + device_printf(dev, "unable to allocate MAC address.\n");
> + return (ENXIO);
> + }
> cvmx_mgmt_port_set_mac(sc->sc_port, mac);
>
> /* No watermark for input ring. */
>
> Modified: head/sys/mips/cavium/octe/ethernet-common.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-common.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-common.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -46,6 +46,8 @@ __FBSDID("$FreeBSD$");
>
> extern int octeon_is_simulation(void);
>
> +static uint64_t mac_addr = 0;
> +static uint32_t mac_offset = 0;
>
> /**
> * Set the multicast list. Currently unimplemented.
> @@ -90,6 +92,37 @@ void cvm_oct_common_set_multicast_list(s
>
>
> /**
> + * Assign a MAC addres from the pool of available MAC addresses
> + * Can return as either a 64-bit value and/or 6 octets.
> + *
> + * @param macp Filled in with the assigned address if non-NULL
> + * @param octets Filled in with the assigned address if non-NULL
> + * @return Zero on success
> + */
> +int cvm_assign_mac_address(uint64_t *macp, uint8_t *octets)
> +{
> + /* Initialize from global MAC address base; fail if not set */
> + if (mac_addr == 0) {
> + memcpy((uint8_t *)&mac_addr + 2, cvmx_sysinfo_get()->mac_addr_base, 6);
> + if (mac_addr == 0)
> + return ENXIO;
> + }
> +
> + if (mac_offset >= cvmx_sysinfo_get()->mac_addr_count)
> + return ENXIO; /* Out of addresses to assign */
> +
> + if (macp)
> + *macp = mac_addr;
> + if (octets)
> + memcpy(octets, (u_int8_t *)&mac_addr + 2, 6);
> +
> + mac_addr++;
> + mac_offset++;
> +
> + return 0;
> +}
> +
> +/**
> * Set the hardware MAC address for a device
> *
> * @param dev Device to change the MAC address for
> @@ -268,16 +301,11 @@ void cvm_oct_common_poll(struct ifnet *i
> */
> int cvm_oct_common_init(struct ifnet *ifp)
> {
> - char mac[6] = {
> - cvmx_sysinfo_get()->mac_addr_base[0],
> - cvmx_sysinfo_get()->mac_addr_base[1],
> - cvmx_sysinfo_get()->mac_addr_base[2],
> - cvmx_sysinfo_get()->mac_addr_base[3],
> - cvmx_sysinfo_get()->mac_addr_base[4],
> - cvmx_sysinfo_get()->mac_addr_base[5] };
> + uint8_t mac[6];
> cvm_oct_private_t *priv = (cvm_oct_private_t *)ifp->if_softc;
>
> - mac[5] += cvm_oct_mac_addr_offset++;
> + if (cvm_assign_mac_address(NULL, mac) != 0)
> + return ENXIO;
>
> ifp->if_mtu = ETHERMTU;
>
>
> Modified: head/sys/mips/cavium/octe/ethernet-common.h
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-common.h Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-common.h Wed Feb 22 01:30:25 2012 (r231987)
> @@ -37,6 +37,7 @@ void cvm_oct_common_uninit(struct ifnet
> int cvm_oct_common_change_mtu(struct ifnet *ifp, int new_mtu);
> void cvm_oct_common_set_multicast_list(struct ifnet *ifp);
> void cvm_oct_common_set_mac_address(struct ifnet *ifp, const void *);
> +int cvm_assign_mac_address(uint64_t *, uint8_t *);
>
> int cvm_oct_init_module(device_t);
> void cvm_oct_cleanup_module(device_t);
> @@ -52,4 +53,3 @@ int cvm_oct_spi_init(struct ifnet *ifp);
> void cvm_oct_spi_uninit(struct ifnet *ifp);
> int cvm_oct_xaui_init(struct ifnet *ifp);
>
> -extern unsigned int cvm_oct_mac_addr_offset;
>
> Modified: head/sys/mips/cavium/octe/ethernet-rgmii.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-rgmii.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-rgmii.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -215,7 +215,9 @@ int cvm_oct_rgmii_init(struct ifnet *ifp
> int error;
> int rid;
>
> - cvm_oct_common_init(ifp);
> + if (cvm_oct_common_init(ifp) != 0)
> + return ENXIO;
> +
> priv->open = cvm_oct_common_open;
> priv->stop = cvm_oct_common_stop;
> priv->stop(ifp);
>
> Modified: head/sys/mips/cavium/octe/ethernet-sgmii.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-sgmii.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-sgmii.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -49,7 +49,10 @@ extern int octeon_is_simulation(void);
> int cvm_oct_sgmii_init(struct ifnet *ifp)
> {
> cvm_oct_private_t *priv = (cvm_oct_private_t *)ifp->if_softc;
> - cvm_oct_common_init(ifp);
> +
> + if (cvm_oct_common_init(ifp) != 0)
> + return ENXIO;
> +
> priv->open = cvm_oct_common_open;
> priv->stop = cvm_oct_common_stop;
> priv->stop(ifp);
>
> Modified: head/sys/mips/cavium/octe/ethernet-spi.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-spi.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-spi.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -289,7 +289,8 @@ int cvm_oct_spi_init(struct ifnet *ifp)
> cvm_oct_spi_enable_error_reporting(INTERFACE(priv->port));
> priv->poll = cvm_oct_spi_poll;
> }
> - cvm_oct_common_init(ifp);
> + if (cvm_oct_common_init(ifp) != 0)
> + return ENXIO;
> return 0;
> }
>
>
> Modified: head/sys/mips/cavium/octe/ethernet-xaui.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet-xaui.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet-xaui.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -49,7 +49,10 @@ extern int octeon_is_simulation(void);
> int cvm_oct_xaui_init(struct ifnet *ifp)
> {
> cvm_oct_private_t *priv = (cvm_oct_private_t *)ifp->if_softc;
> - cvm_oct_common_init(ifp);
> +
> + if (cvm_oct_common_init(ifp) != 0)
> + return ENXIO;
> +
> priv->open = cvm_oct_common_open;
> priv->stop = cvm_oct_common_stop;
> priv->stop(ifp);
>
> Modified: head/sys/mips/cavium/octe/ethernet.c
> ==============================================================================
> --- head/sys/mips/cavium/octe/ethernet.c Wed Feb 22 01:23:14 2012 (r231986)
> +++ head/sys/mips/cavium/octe/ethernet.c Wed Feb 22 01:30:25 2012 (r231987)
> @@ -97,15 +97,6 @@ static struct taskqueue *cvm_oct_link_ta
> */
> static int cvm_oct_num_output_buffers;
>
> -/*
> - * The offset from mac_addr_base that should be used for the next port
> - * that is configured. By convention, if any mgmt ports exist on the
> - * chip, they get the first mac addresses. The ports controlled by
> - * this driver are numbered sequencially following any mgmt addresses
> - * that may exist.
> - */
> -unsigned int cvm_oct_mac_addr_offset;
> -
> /**
> * Function to update link status.
> */
> @@ -321,20 +312,6 @@ int cvm_oct_init_module(device_t bus)
>
> printf("cavium-ethernet: %s\n", OCTEON_SDK_VERSION_STRING);
>
> - /*
> - * MAC addresses for this driver start after the management
> - * ports.
> - *
> - * XXX Would be nice if __cvmx_mgmt_port_num_ports() were
> - * not static to cvmx-mgmt-port.c.
> - */
> - if (OCTEON_IS_MODEL(OCTEON_CN56XX))
> - cvm_oct_mac_addr_offset = 1;
> - else if (OCTEON_IS_MODEL(OCTEON_CN52XX) || OCTEON_IS_MODEL(OCTEON_CN63XX))
> - cvm_oct_mac_addr_offset = 2;
> - else
> - cvm_oct_mac_addr_offset = 0;
> -
> cvm_oct_rx_initialize();
> cvm_oct_configure_common_hw(bus);
>
> @@ -375,15 +352,17 @@ int cvm_oct_init_module(device_t bus)
> int num_ports = cvmx_helper_ports_on_interface(interface);
> int port;
>
> - for (port = cvmx_helper_get_ipd_port(interface, 0); port < cvmx_helper_get_ipd_port(interface, num_ports); port++) {
> + for (port = cvmx_helper_get_ipd_port(interface, 0);
> + port < cvmx_helper_get_ipd_port(interface, num_ports);
> + ifnum++, port++) {
> cvm_oct_private_t *priv;
> struct ifnet *ifp;
>
> - dev = BUS_ADD_CHILD(bus, 0, "octe", ifnum++);
> + dev = BUS_ADD_CHILD(bus, 0, "octe", ifnum);
> if (dev != NULL)
> ifp = if_alloc(IFT_ETHER);
> if (dev == NULL || ifp == NULL) {
> - printf("\t\tFailed to allocate ethernet device for port %d\n", port);
> + printf("Failed to allocate ethernet device for interface %d port %d\n", interface, port);
> continue;
> }
>
> @@ -454,12 +433,13 @@ int cvm_oct_init_module(device_t bus)
> ifp->if_softc = priv;
>
> if (!priv->init) {
> - panic("%s: unsupported device type, need to free ifp.", __func__);
> - } else
> - if (priv->init(ifp) < 0) {
> - printf("\t\tFailed to register ethernet device for interface %d, port %d\n",
> - interface, priv->port);
> - panic("%s: init failed, need to free ifp.", __func__);
> + printf("octe%d: unsupported device type interface %d, port %d\n",
> + ifnum, interface, priv->port);
> + if_free(ifp);
> + } else if (priv->init(ifp) != 0) {
> + printf("octe%d: failed to register device for interface %d, port %d\n",
> + ifnum, interface, priv->port);
> + if_free(ifp);
> } else {
> cvm_oct_device[priv->port] = ifp;
> fau -= cvmx_pko_get_num_queues(priv->port) * sizeof(uint32_t);
More information about the svn-src-head
mailing list