Changeset View
Standalone View
sys/net/if_bridge.c
Show First 20 Lines • Show All 684 Lines • ▼ Show 20 Lines | |||||
* | * | ||||
* Create a new bridge instance. | * Create a new bridge instance. | ||||
*/ | */ | ||||
static int | static int | ||||
bridge_clone_create(struct if_clone *ifc, int unit, caddr_t params) | bridge_clone_create(struct if_clone *ifc, int unit, caddr_t params) | ||||
{ | { | ||||
struct bridge_softc *sc, *sc2; | struct bridge_softc *sc, *sc2; | ||||
struct ifnet *bifp, *ifp; | struct ifnet *bifp, *ifp; | ||||
int fb, retry; | int retry; | ||||
unsigned long hostid; | unsigned long hostid; | ||||
sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO); | sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO); | ||||
ifp = sc->sc_ifp = if_alloc(IFT_ETHER); | ifp = sc->sc_ifp = if_alloc(IFT_ETHER); | ||||
if (ifp == NULL) { | if (ifp == NULL) { | ||||
free(sc, M_DEVBUF); | free(sc, M_DEVBUF); | ||||
return (ENOSPC); | return (ENOSPC); | ||||
} | } | ||||
Show All 15 Lines | bridge_clone_create(struct if_clone *ifc, int unit, caddr_t params) | ||||
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; | ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; | ||||
ifp->if_ioctl = bridge_ioctl; | ifp->if_ioctl = bridge_ioctl; | ||||
ifp->if_transmit = bridge_transmit; | ifp->if_transmit = bridge_transmit; | ||||
ifp->if_qflush = bridge_qflush; | ifp->if_qflush = bridge_qflush; | ||||
ifp->if_init = bridge_init; | ifp->if_init = bridge_init; | ||||
ifp->if_type = IFT_BRIDGE; | ifp->if_type = IFT_BRIDGE; | ||||
/* | /* | ||||
* Generate an ethernet address with a locally administered address. | * Generate an ethernet address. | ||||
* | * | ||||
* Since we are using random ethernet addresses for the bridge, it is | * Since we are using random ethernet addresses for the bridge, it is | ||||
* possible that we might have address collisions, so make sure that | * possible that we might have address collisions, so make sure that | ||||
* this hardware address isn't already in use on another bridge. | * this hardware address isn't already in use on another bridge. | ||||
* The first try uses the hostid and falls back to arc4rand(). | |||||
*/ | */ | ||||
fb = 0; | |||||
getcredhostid(curthread->td_ucred, &hostid); | getcredhostid(curthread->td_ucred, &hostid); | ||||
do { | do { | ||||
if (fb || hostid == 0) { | |||||
ether_gen_addr(ifp, &sc->sc_defaddr); | ether_gen_addr(ifp, &sc->sc_defaddr); | ||||
} else { | |||||
sc->sc_defaddr.octet[0] = 0x2; | |||||
sc->sc_defaddr.octet[1] = (hostid >> 24) & 0xff; | |||||
sc->sc_defaddr.octet[2] = (hostid >> 16) & 0xff; | |||||
sc->sc_defaddr.octet[3] = (hostid >> 8 ) & 0xff; | |||||
sc->sc_defaddr.octet[4] = hostid & 0xff; | |||||
sc->sc_defaddr.octet[5] = ifp->if_dunit & 0xff; | |||||
} | |||||
fb = 1; | |||||
retry = 0; | retry = 0; | ||||
BRIDGE_LIST_LOCK(); | BRIDGE_LIST_LOCK(); | ||||
LIST_FOREACH(sc2, &V_bridge_list, sc_list) { | LIST_FOREACH(sc2, &V_bridge_list, sc_list) { | ||||
bifp = sc2->sc_ifp; | bifp = sc2->sc_ifp; | ||||
if (memcmp(sc->sc_defaddr.octet, | if (memcmp(sc->sc_defaddr.octet, | ||||
IF_LLADDR(bifp), ETHER_ADDR_LEN) == 0) { | IF_LLADDR(bifp), ETHER_ADDR_LEN) == 0) { | ||||
retry = 1; | retry = 1; | ||||
break; | break; | ||||
} | } | ||||
} | } | ||||
BRIDGE_LIST_UNLOCK(); | BRIDGE_LIST_UNLOCK(); | ||||
} while (retry == 1); | } while (retry == 1); | ||||
bz: I don't fully understand the logic but ether_gen_addr() will always give you the same result in… | |||||
kevansUnsubmitted Not Done Inline ActionsAgreed... I suspect the loop and check for a conflict should probably just go away or get fixed (and not loop unless randomness comes back). It's really not (and wasn't) enough to just check bridge interfaces because those aren't the only kind of interfaces that get generated/random/arbitrary MAC addresses. It also seems like it would've been safer to assume that other vnets on the same machine /could/ be in the same broadcast domain and check bridges in all vnets. kevans: Agreed... I suspect the loop and check for a conflict should probably just go away or get fixed… | |||||
bzUnsubmitted Not Done Inline ActionsOr for that one of the bridges in one of the 20 machines next to it in the rack running all the same with some similarly broken non-unique setup. bz: Or for that one of the bridges in one of the 20 machines next to it in the rack running all the… | |||||
kpAuthorUnsubmitted Done Inline ActionsYeah, that's a great point. We must get rid of the loop here. kp: Yeah, that's a great point. We must get rid of the loop here. | |||||
bstp_attach(&sc->sc_stp, &bridge_ops); | bstp_attach(&sc->sc_stp, &bridge_ops); | ||||
ether_ifattach(ifp, sc->sc_defaddr.octet); | ether_ifattach(ifp, sc->sc_defaddr.octet); | ||||
/* Now undo some of the damage... */ | /* Now undo some of the damage... */ | ||||
ifp->if_baudrate = 0; | ifp->if_baudrate = 0; | ||||
ifp->if_type = IFT_BRIDGE; | ifp->if_type = IFT_BRIDGE; | ||||
BRIDGE_LIST_LOCK(); | BRIDGE_LIST_LOCK(); | ||||
▲ Show 20 Lines • Show All 2,993 Lines • Show Last 20 Lines |
I don't fully understand the logic but ether_gen_addr() will always give you the same result in the loop so the outcome of the retry is not going to change, right? Do we need the do { } while() loop still or is that an endless loop now and we should rather have proper error handling? I am not even sure this was sane before... But that's probably a different question to raise.