Cole Robinson wrote: > Fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=235961 > > If using the default virtual network, an easy way to lose guest network > connectivity is to install libvirt inside the VM. The autostarted > default network inside the guest collides with host virtual network > routing. This is a long standing issue that has caused users quite a > bit of pain and confusion. > > On network startup, parse /proc/net/route and compare the requested > IP+netmask against host routing destinations: if any matches are found, > refuse to start the network. > > v2: Drop sscanf, fix a comment typo, comment that function could use > libnl instead of /proc > > v3: Consider route netmask. Compare binary data rather than convert to > string. ... > +static int networkCheckRouteCollision(virNetworkObjPtr network) > +{ > + int ret = -1, len; > + char *cur, *buf = NULL; > + enum {MAX_ROUTE_SIZE = 1024*64}; > + struct in_addr inaddress, innetmask; > + char netaddr[32]; > + > + if (!network->def->ipAddress || !network->def->netmask) > + return 0; > + > + if (inet_pton(AF_INET, network->def->ipAddress, &inaddress) <= 0) { > + networkReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse IP address '%s'"), > + network->def->ipAddress); > + goto error; > + } > + if (inet_pton(AF_INET, network->def->netmask, &innetmask) <= 0) { > + networkReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse netmask '%s'"), > + network->def->netmask); > + goto error; > + } > + > + inaddress.s_addr &= innetmask.s_addr; > + if (!inet_ntop(AF_INET, &inaddress, netaddr, sizeof(netaddr))) { > + virReportSystemError(errno, "%s", > + _("failed to format network address")); > + goto error; > + } > + > + /* Read whole routing table into memory */ > + if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0) > + goto error; > + /* Dropping the last character shouldn't hurt */ Hi Cole, Not sure how much you want to invest in manual parsing, but in case this code ends up staying with us for a while, here are some suggestions. Handle the case in which the file is empty. This will appease static checkers like clang and coverity. Either change the "< 0" test above to "<= 0" or do e.g., if (len > 0) > + buf[len-1] = '\0'; Future proof it by skipping that first line only if it looks like the heading we see now: if (STRPREFIX (buf, "Iface")) > + /* First line is just headings, skip it */ > + cur = strchr(buf, '\n'); > + > + while (cur) { > + char *data[8]; > + char *dest, *iface, *mask; > + unsigned int addr_val, mask_val; > + int i; It's slightly better to make "i" unsigned. The following assumes no leading white space, which is currently true at least on F13. Future/portability-proof it by skipping leading white space: while (c_isblank (*cur)) cur++; > + cur++; > + > + /* Delimit interface field */ > + for (i = 0; i < sizeof(data); ++i) { Oops. Won't this make "i" iterate from 0 up to 31 or 63, depending on sizeof char*? You probably mean this: for (i = 0; i < ARRAY_CARDINALITY(data); ++i) { > + data[i] = cur; Otherwise, this would overrun the 8-element "data" array and clobber the stack. > + /* Parse fields and delimit */ > + while(*cur > ' ') { Using "> ' '" works as long as each line has 8 or more fields. If there are fewer, it would treat the newline just like a regular field separator and continue getting fields from the next line. If you use c_isblank you have to handle end of line differently, but that's a good thing. How about using c_isblank here, too, rather than relying on ASCII "space" being smaller than "interesting" byte codes? On F13, the fields of /proc/net/route are TAB-delimited. Besides, I think y while (*cur && !c_isblank (*cur)) { > + cur++; > + } > + *cur++ = '\0'; You'll want something here to keep "cur" from going more than 1 beyond the end of the buffer, in case the last row has fewer than 8 columns. > + } > + > + iface = data[0]; > + dest = data[1]; > + mask = data[7]; If a line had fewer than 8 columns, at least mask is not valid. Similarly for dest if there are fewer than two columns. > + if (virStrToLong_ui(dest, NULL, 16, &addr_val) < 0) { > + networkReportError(VIR_ERR_INTERNAL_ERROR, > + _("Failed to convert network address %s"), > + dest); > + goto error; > + } -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list