Page MenuHomeFreeBSD
Differential D23342 Diff 67229 usr.sbin/bhyve/pci_virtio_net.c

Changeset View

Standalone View

View Options

usr.sbin/bhyve/pci_virtio_net.c

Show First 20 Lines • Show All 245 Lines • ▼ Show 20 Lines do {
break; break;
} }
info[n_chains].len = (uint32_t)count_iov(cur_iov, n); info[n_chains].len = (uint32_t)count_iov(cur_iov, n);
cur_iov_bytes += info[n_chains].len; cur_iov_bytes += info[n_chains].len;
cur_iov += n; cur_iov += n;
n_chains++; n_chains++;
} while (cur_iov_bytes < VTNET_MAX_PKT_LEN && } while (cur_iov_bytes < VTNET_MAX_PKT_LEN &&
cur_iov_len < VTNET_MAXSEGS); cur_iov_len < VTNET_MAXSEGS);
afedorovUnsubmitted
Done
Inline Actions

We need to initialize 'struct virtio_net_rxhdr *hdr = iov[0].iov_base;' before calling netbe_recv() which can change value of iov[0].iov_base.

afedorov: We need to initialize 'struct virtio_net_rxhdr *hdr = iov[0].iov_base;' before calling…
vmaffioneAuthorUnsubmitted
Done
Inline Actions

You're right, thank you!

vmaffione: You're right, thank you!
len = netbe_recv(sc->vsc_be, iov, cur_iov_len); len = netbe_recv(sc->vsc_be, iov, cur_iov_len);
if (len <= 0) { if (len <= 0) {
/* /*
* No more packets (len == 0), or backend errored * No more packets (len == 0), or backend errored
* (err < 0). Return unused available buffers * (err < 0). Return unused available buffers
* and stop. * and stop.
*/ */
vq_retchains(vq, n_chains); vq_retchains(vq, n_chains);
/* Interrupt if needed/appropriate and stop. */ /* Interrupt if needed/appropriate and stop. */
vq_endchains(vq, /*used_all_avail=*/0); vq_endchains(vq, /*used_all_avail=*/0);
return; return;
} }
ulen = (uint32_t)len; /* avoid too many casts below */ ulen = (uint32_t)len; /* avoid too many casts below */
/* Publish the used buffers to the guest. */ /* Publish the used buffers to the guest. */
if (!sc->rx_merge) { if (!sc->rx_merge) {
vq_relchain(vq, info[0].idx, ulen); vq_relchain(vq, info[0].idx, ulen);
} else { } else {
struct virtio_net_rxhdr *hdr = iov[0].iov_base; struct virtio_net_rxhdr *hdr = iov[0].iov_base;
afedorovUnsubmitted
Done
Inline Actions

I tried to test this patch. And I found a bug.
With a tap backend, the netbe_recv() function calls iov_trim() which changes the value of iov[0].iov_base (It increases the address by the size of the struct virtio_net_rxhdr).
https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/net_backends.c?view=markup#l819
https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/net_backends.c?view=markup#l766

So, hdr points to an incorrect location.

If I move the 'hdr' declaration before calling the netbe_recv () function, then everything works correctly:
ping -f -s 8000 and iperf3 with MTU == 9000.

afedorov: I tried to test this patch. And I found a bug. With a tap backend, the netbe_recv() function…
vmaffioneAuthorUnsubmitted
Done
Inline Actions

Thank you for the suggestion. I will try to take a different approach to avoid visible changes to the iov

vmaffione: Thank you for the suggestion. I will try to take a different approach to avoid visible changes…
uint32_t iolen; uint32_t iolen;
int i = 0; int i = 0;
assert(iov[0].iov_len >= sizeof(*hdr)); assert(iov[0].iov_len >= sizeof(*hdr));
do { do {
iolen = info[i].len; iolen = info[i].len;
if (iolen > ulen) { if (iolen > ulen) {
▲ Show 20 Lines • Show All 175 Lines • ▼ Show 20 Lines
#endif #endif
/* /*
* Attempt to open the backend device and read the MAC address * Attempt to open the backend device and read the MAC address
* if specified. * if specified.
*/ */
mac_provided = 0; mac_provided = 0;
if (opts != NULL) { if (opts != NULL) {
int mrg_rxbuf_enabled = 0;
int mrg_rxbuf_disabled = 0;
char *devname; char *devname;
char *vtopts; char *vtopts;
int err; int err = 0;
/* Get the device name. */
devname = vtopts = strdup(opts); devname = vtopts = strdup(opts);
(void) strsep(&vtopts, ","); (void) strsep(&vtopts, ",");
if (vtopts != NULL) { /*
err = net_parsemac(vtopts, sc->vsc_config.mac); * Parse the list of options in the form
if (err != 0) { * key1=value1,...,keyN=valueN.
*/
while (vtopts != NULL) {
char *value = vtopts;
char *key;
key = strsep(&value, "=");
if (value == NULL) {
break;
}
vtopts = value;
(void) strsep(&vtopts, ",");
if (strcmp(key, "mac") == 0) {
err = net_parsemac(value, sc->vsc_config.mac);
if (err) {
break;
}
mac_provided = 1;
} else if (strcmp(key, "mrgrxbuf") == 0) {
if (strcmp(value, "on") == 0) {
mrg_rxbuf_enabled = 1;
mrg_rxbuf_disabled = 0;
} else {
mrg_rxbuf_disabled = 1;
mrg_rxbuf_enabled = 0;
}
}
}
if (err) {
free(devname); free(devname);
free(sc); free(sc);
return (err); return (err);
} }
mac_provided = 1;
}
err = netbe_init(&sc->vsc_be, devname, pci_vtnet_rx_callback, err = netbe_init(&sc->vsc_be, devname, pci_vtnet_rx_callback,
sc); sc);
free(devname); free(devname);
if (err) { if (err) {
free(sc); free(sc);
return (err); return (err);
} }
sc->vsc_consts.vc_hv_caps |= netbe_get_cap(sc->vsc_be); sc->vsc_consts.vc_hv_caps |= netbe_get_cap(sc->vsc_be);
if (mrg_rxbuf_enabled)
sc->vsc_consts.vc_hv_caps |= VIRTIO_NET_F_MRG_RXBUF;
else if (mrg_rxbuf_disabled)
sc->vsc_consts.vc_hv_caps &= ~VIRTIO_NET_F_MRG_RXBUF;
} }
if (!mac_provided) { if (!mac_provided) {
net_genmac(pi, sc->vsc_config.mac); net_genmac(pi, sc->vsc_config.mac);
} }
/* initialize config space */ /* initialize config space */
pci_set_cfgdata16(pi, PCIR_DEVICE, VIRTIO_DEV_NET); pci_set_cfgdata16(pi, PCIR_DEVICE, VIRTIO_DEV_NET);
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines