| Message ID | 20190725032321.12721-14-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add virtual device fuzzing support | expand |
On 7/24/19 11:23 PM, Oleinik, Alexander wrote: > Signed-off-by: Alexander Oleinik <alxndr@bu.edu> Is there some explanation for why the below patch does what the subject line claims for the uninitiated? I don't know why increasing the number of queues from 2 to 3 here is correct in the general case, OR why it would "add ctrl vq support". (Or what it has to do with fuzzing, in general.) [Only responding because this landed in tests/libqos, which I do try to keep an eye on, but this patch is opaque to me. --js] > --- > tests/libqos/virtio-net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/libqos/virtio-net.c b/tests/libqos/virtio-net.c > index 66405b646e..247a0a17a8 100644 > --- a/tests/libqos/virtio-net.c > +++ b/tests/libqos/virtio-net.c > @@ -51,7 +51,7 @@ static void virtio_net_setup(QVirtioNet *interface) > if (features & (1u << VIRTIO_NET_F_MQ)) { > interface->n_queues = qvirtio_config_readw(vdev, 8) * 2; > } else { > - interface->n_queues = 2; > + interface->n_queues = 3; > } > > interface->queues = g_new(QVirtQueue *, interface->n_queues); >
On Thu, 2019-07-25 at 12:25 -0400, John Snow wrote: > > On 7/24/19 11:23 PM, Oleinik, Alexander wrote: > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu> > > Is there some explanation for why the below patch does what the > subject > line claims for the uninitiated? When multiqueue mode (VIRTIO_NET_F_MQ) is disabled, virtio-net sets up three queues. 0:receiveq, 1:transmitq and 2:controlq. > I don't know why increasing the number of queues from 2 to 3 here is > correct in the general case, OR why it would "add ctrl vq support". > (Or what it has to do with fuzzing, in general.) Prior to the change, accessing the ctrl vq through QOS, would trigger a segfault, since only two queues were allocated to QVirtioDevice* interface->queues. Also, when VIRTIO_NET_F_MQ is enabled, the number of queues is 2*N + 1, so I think in that case n->n_queues is also short by one in the code below. > [Only responding because this landed in tests/libqos, which I do try > to > keep an eye on, but this patch is opaque to me. --js] > > > --- > > tests/libqos/virtio-net.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/libqos/virtio-net.c b/tests/libqos/virtio-net.c > > index 66405b646e..247a0a17a8 100644 > > --- a/tests/libqos/virtio-net.c > > +++ b/tests/libqos/virtio-net.c > > @@ -51,7 +51,7 @@ static void virtio_net_setup(QVirtioNet > > *interface) > > if (features & (1u << VIRTIO_NET_F_MQ)) { > > interface->n_queues = qvirtio_config_readw(vdev, 8) * 2; > > } else { > > - interface->n_queues = 2; > > + interface->n_queues = 3; > > } > > > > interface->queues = g_new(QVirtQueue *, interface->n_queues); > >
On Thu, Jul 25, 2019 at 05:05:25PM +0000, Oleinik, Alexander wrote: > On Thu, 2019-07-25 at 12:25 -0400, John Snow wrote: > > > > On 7/24/19 11:23 PM, Oleinik, Alexander wrote: > > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu> > > > > Is there some explanation for why the below patch does what the > > subject > > line claims for the uninitiated? > When multiqueue mode (VIRTIO_NET_F_MQ) is disabled, virtio-net sets up > three queues. 0:receiveq, 1:transmitq and 2:controlq. > > I don't know why increasing the number of queues from 2 to 3 here is > > correct in the general case, OR why it would "add ctrl vq support". > > (Or what it has to do with fuzzing, in general.) > > Prior to the change, accessing the ctrl vq through QOS, would trigger a > segfault, since only two queues were allocated to QVirtioDevice* > interface->queues. > > Also, when VIRTIO_NET_F_MQ is enabled, the number of queues is 2*N + 1, > so I think in that case n->n_queues is also short by one in the code > below. I think the patch could be changed to: > > [Only responding because this landed in tests/libqos, which I do try > > to > > keep an eye on, but this patch is opaque to me. --js] > > > > > --- > > > tests/libqos/virtio-net.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/libqos/virtio-net.c b/tests/libqos/virtio-net.c > > > index 66405b646e..247a0a17a8 100644 > > > --- a/tests/libqos/virtio-net.c > > > +++ b/tests/libqos/virtio-net.c > > > @@ -51,7 +51,7 @@ static void virtio_net_setup(QVirtioNet > > > *interface) > > > if (features & (1u << VIRTIO_NET_F_MQ)) { > > > interface->n_queues = qvirtio_config_readw(vdev, 8) * 2; > > > } else { > > > - interface->n_queues = 2; > > > + interface->n_queues = 3; > > > } interface->n_queues++; /* ctrl vq */ And a comment added to the QVirtQueue::n_queues field definition: /* total number of virtqueues (rx, tx, ctrl) */ This will prevent confusion about whether the ctrl queue is counted or not.
diff --git a/tests/libqos/virtio-net.c b/tests/libqos/virtio-net.c index 66405b646e..247a0a17a8 100644 --- a/tests/libqos/virtio-net.c +++ b/tests/libqos/virtio-net.c @@ -51,7 +51,7 @@ static void virtio_net_setup(QVirtioNet *interface) if (features & (1u << VIRTIO_NET_F_MQ)) { interface->n_queues = qvirtio_config_readw(vdev, 8) * 2; } else { - interface->n_queues = 2; + interface->n_queues = 3; } interface->queues = g_new(QVirtQueue *, interface->n_queues);
Signed-off-by: Alexander Oleinik <alxndr@bu.edu> --- tests/libqos/virtio-net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)