| Message ID | 20231017040932.62997-5-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-net RSS/hash report fixes and improvements | expand |
On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > While netmap implements virtio-net header, it does not implement > receive_raw(). The only user for raw is the announcing. Netmap probably doesn't it at all. > Instead of implementing receive_raw for netmap, add > virtio-net headers in the common code and use receive_iov()/receive() > instead. This also fixes the buffer size for the virtio-net header. > > Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > include/net/net.h | 1 - > include/net/queue.h | 1 - > net/net.c | 17 +++++++++-------- > net/queue.c | 30 ++++++++++-------------------- > net/tap.c | 1 - > 5 files changed, 19 insertions(+), 31 deletions(-) > > diff --git a/include/net/net.h b/include/net/net.h > index 011031ef77..971dc54897 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -71,7 +71,6 @@ typedef struct NetClientInfo { > NetClientDriver type; > size_t size; > NetReceive *receive; > - NetReceive *receive_raw; > NetReceiveIOV *receive_iov; > NetCanReceive *can_receive; > NetStart *start; > diff --git a/include/net/queue.h b/include/net/queue.h > index 9f2f289d77..7a43863be2 100644 > --- a/include/net/queue.h > +++ b/include/net/queue.h > @@ -31,7 +31,6 @@ typedef struct NetQueue NetQueue; > typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret); > > #define QEMU_NET_PACKET_FLAG_NONE 0 > -#define QEMU_NET_PACKET_FLAG_RAW (1<<0) > > /* Returns: > * >0 - success > diff --git a/net/net.c b/net/net.c > index 6d2fa8d40f..2d94d23c07 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -752,8 +752,13 @@ ssize_t qemu_receive_packet_iov(NetClientState *nc, const struct iovec *iov, > > ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) > { > - return qemu_send_packet_async_with_flags(nc, QEMU_NET_PACKET_FLAG_RAW, > - buf, size, NULL); > + struct virtio_net_hdr_v1_hash vnet_hdr = { }; > + struct iovec iov[] = { > + { .iov_base = &vnet_hdr, .iov_len = nc->vnet_hdr_len }, > + { .iov_base = (void *)buf, .iov_len = size } > + }; Having virtio-net specific code in the net layer is a layer violation. I'd leave it to tap. Thanks
On 2023/10/27 15:49, Jason Wang wrote: > On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: >> >> While netmap implements virtio-net header, it does not implement >> receive_raw(). > > The only user for raw is the announcing. Netmap probably doesn't it at all. In my understanding, the announcing *sends* a raw packet. Both of tap and netmap *receive* packets with virtio-net headers. Regards, Akihiko Odaki
在 2023/10/27 15:52, Akihiko Odaki 写道: > On 2023/10/27 15:49, Jason Wang wrote: >> On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki >> <akihiko.odaki@daynix.com> wrote: >>> >>> While netmap implements virtio-net header, it does not implement >>> receive_raw(). >> >> The only user for raw is the announcing. Netmap probably doesn't it >> at all. > > In my understanding, the announcing *sends* a raw packet. It's send via NIC and receive by its peer which is the TAP qemu_send_packet_raw() -> nc -> nc->peer -> peer->receive_raw()? Anything I miss? Thanks > Both of tap and netmap *receive* packets with virtio-net headers. > > Regards, > Akihiko Odaki >
On 2023/10/30 12:06, Jason Wang wrote: > > 在 2023/10/27 15:52, Akihiko Odaki 写道: >> On 2023/10/27 15:49, Jason Wang wrote: >>> On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki >>> <akihiko.odaki@daynix.com> wrote: >>>> >>>> While netmap implements virtio-net header, it does not implement >>>> receive_raw(). >>> >>> The only user for raw is the announcing. Netmap probably doesn't it >>> at all. >> >> In my understanding, the announcing *sends* a raw packet. > > > It's send via NIC and receive by its peer which is the TAP > > qemu_send_packet_raw() -> nc -> nc->peer -> peer->receive_raw()? > > Anything I miss? The problem is that the peer can be netmap and netmap also requires a virtio-net header. Regards, Akihiko Odaki
On Mon, Oct 30, 2023 at 12:03 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > On 2023/10/30 12:06, Jason Wang wrote: > > > > 在 2023/10/27 15:52, Akihiko Odaki 写道: > >> On 2023/10/27 15:49, Jason Wang wrote: > >>> On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki > >>> <akihiko.odaki@daynix.com> wrote: > >>>> > >>>> While netmap implements virtio-net header, it does not implement > >>>> receive_raw(). > >>> > >>> The only user for raw is the announcing. Netmap probably doesn't it > >>> at all. > >> > >> In my understanding, the announcing *sends* a raw packet. > > > > > > It's send via NIC and receive by its peer which is the TAP > > > > qemu_send_packet_raw() -> nc -> nc->peer -> peer->receive_raw()? > > > > Anything I miss? > > The problem is that the peer can be netmap and netmap also requires a > virtio-net header. Right, but I don't know whether netmap can migrate. Thanks > > Regards, > Akihiko Odaki >
On 2023/10/30 13:08, Jason Wang wrote: > On Mon, Oct 30, 2023 at 12:03 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: >> >> On 2023/10/30 12:06, Jason Wang wrote: >>> >>> 在 2023/10/27 15:52, Akihiko Odaki 写道: >>>> On 2023/10/27 15:49, Jason Wang wrote: >>>>> On Tue, Oct 17, 2023 at 12:09 PM Akihiko Odaki >>>>> <akihiko.odaki@daynix.com> wrote: >>>>>> >>>>>> While netmap implements virtio-net header, it does not implement >>>>>> receive_raw(). >>>>> >>>>> The only user for raw is the announcing. Netmap probably doesn't it >>>>> at all. >>>> >>>> In my understanding, the announcing *sends* a raw packet. >>> >>> >>> It's send via NIC and receive by its peer which is the TAP >>> >>> qemu_send_packet_raw() -> nc -> nc->peer -> peer->receive_raw()? >>> >>> Anything I miss? >> >> The problem is that the peer can be netmap and netmap also requires a >> virtio-net header. > > Right, but I don't know whether netmap can migrate. Thinking of the condition that announcement can happen, I'm not aware anything that prevents migration with netamp. It also is apparently possible to make an announcement with HMP/QMP. In any case, I think it's better to fix qemu_send_packet_raw() for netmap to prevent potential breakage especially if it costs nothing (actually it saves some code). Regards, Akihiko Odaki
diff --git a/include/net/net.h b/include/net/net.h index 011031ef77..971dc54897 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -71,7 +71,6 @@ typedef struct NetClientInfo { NetClientDriver type; size_t size; NetReceive *receive; - NetReceive *receive_raw; NetReceiveIOV *receive_iov; NetCanReceive *can_receive; NetStart *start; diff --git a/include/net/queue.h b/include/net/queue.h index 9f2f289d77..7a43863be2 100644 --- a/include/net/queue.h +++ b/include/net/queue.h @@ -31,7 +31,6 @@ typedef struct NetQueue NetQueue; typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret); #define QEMU_NET_PACKET_FLAG_NONE 0 -#define QEMU_NET_PACKET_FLAG_RAW (1<<0) /* Returns: * >0 - success diff --git a/net/net.c b/net/net.c index 6d2fa8d40f..2d94d23c07 100644 --- a/net/net.c +++ b/net/net.c @@ -752,8 +752,13 @@ ssize_t qemu_receive_packet_iov(NetClientState *nc, const struct iovec *iov, ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) { - return qemu_send_packet_async_with_flags(nc, QEMU_NET_PACKET_FLAG_RAW, - buf, size, NULL); + struct virtio_net_hdr_v1_hash vnet_hdr = { }; + struct iovec iov[] = { + { .iov_base = &vnet_hdr, .iov_len = nc->vnet_hdr_len }, + { .iov_base = (void *)buf, .iov_len = size } + }; + + return qemu_sendv_packet_async(nc, iov, ARRAY_SIZE(iov), NULL); } static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, @@ -777,11 +782,7 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, offset = iov_to_buf(iov, iovcnt, 0, buf, offset); } - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { - ret = nc->info->receive_raw(nc, buffer, offset); - } else { - ret = nc->info->receive(nc, buffer, offset); - } + ret = nc->info->receive(nc, buffer, offset); g_free(buf); return ret; @@ -814,7 +815,7 @@ static ssize_t qemu_deliver_packet_iov(NetClientState *sender, owned_reentrancy_guard->engaged_in_io = true; } - if (nc->info->receive_iov && !(flags & QEMU_NET_PACKET_FLAG_RAW)) { + if (nc->info->receive_iov) { ret = nc->info->receive_iov(nc, iov, iovcnt); } else { ret = nc_sendv_compat(nc, iov, iovcnt, flags); diff --git a/net/queue.c b/net/queue.c index c872d51df8..70d29d7ac0 100644 --- a/net/queue.c +++ b/net/queue.c @@ -43,7 +43,6 @@ struct NetPacket { QTAILQ_ENTRY(NetPacket) entry; NetClientState *sender; - unsigned flags; int size; NetPacketSent *sent_cb; uint8_t data[]; @@ -92,7 +91,6 @@ void qemu_del_net_queue(NetQueue *queue) static void qemu_net_queue_append(NetQueue *queue, NetClientState *sender, - unsigned flags, const uint8_t *buf, size_t size, NetPacketSent *sent_cb) @@ -104,7 +102,6 @@ static void qemu_net_queue_append(NetQueue *queue, } packet = g_malloc(sizeof(NetPacket) + size); packet->sender = sender; - packet->flags = flags; packet->size = size; packet->sent_cb = sent_cb; memcpy(packet->data, buf, size); @@ -115,7 +112,6 @@ static void qemu_net_queue_append(NetQueue *queue, void qemu_net_queue_append_iov(NetQueue *queue, NetClientState *sender, - unsigned flags, const struct iovec *iov, int iovcnt, NetPacketSent *sent_cb) @@ -134,7 +130,6 @@ void qemu_net_queue_append_iov(NetQueue *queue, packet = g_malloc(sizeof(NetPacket) + max_len); packet->sender = sender; packet->sent_cb = sent_cb; - packet->flags = flags; packet->size = 0; for (i = 0; i < iovcnt; i++) { @@ -150,7 +145,6 @@ void qemu_net_queue_append_iov(NetQueue *queue, static ssize_t qemu_net_queue_deliver(NetQueue *queue, NetClientState *sender, - unsigned flags, const uint8_t *data, size_t size) { @@ -161,7 +155,7 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, }; queue->delivering = 1; - ret = queue->deliver(sender, flags, &iov, 1, queue->opaque); + ret = queue->deliver(sender, &iov, 1, queue->opaque); queue->delivering = 0; return ret; @@ -169,14 +163,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, static ssize_t qemu_net_queue_deliver_iov(NetQueue *queue, NetClientState *sender, - unsigned flags, const struct iovec *iov, int iovcnt) { ssize_t ret = -1; queue->delivering = 1; - ret = queue->deliver(sender, flags, iov, iovcnt, queue->opaque); + ret = queue->deliver(sender, iov, iovcnt, queue->opaque); queue->delivering = 0; return ret; @@ -190,7 +183,7 @@ ssize_t qemu_net_queue_receive(NetQueue *queue, return 0; } - return qemu_net_queue_deliver(queue, NULL, 0, data, size); + return qemu_net_queue_deliver(queue, NULL, data, size); } ssize_t qemu_net_queue_receive_iov(NetQueue *queue, @@ -201,12 +194,11 @@ ssize_t qemu_net_queue_receive_iov(NetQueue *queue, return 0; } - return qemu_net_queue_deliver_iov(queue, NULL, 0, iov, iovcnt); + return qemu_net_queue_deliver_iov(queue, NULL, iov, iovcnt); } ssize_t qemu_net_queue_send(NetQueue *queue, NetClientState *sender, - unsigned flags, const uint8_t *data, size_t size, NetPacketSent *sent_cb) @@ -214,13 +206,13 @@ ssize_t qemu_net_queue_send(NetQueue *queue, ssize_t ret; if (queue->delivering || !qemu_can_send_packet(sender)) { - qemu_net_queue_append(queue, sender, flags, data, size, sent_cb); + qemu_net_queue_append(queue, sender, data, size, sent_cb); return 0; } - ret = qemu_net_queue_deliver(queue, sender, flags, data, size); + ret = qemu_net_queue_deliver(queue, sender, data, size); if (ret == 0) { - qemu_net_queue_append(queue, sender, flags, data, size, sent_cb); + qemu_net_queue_append(queue, sender, data, size, sent_cb); return 0; } @@ -231,7 +223,6 @@ ssize_t qemu_net_queue_send(NetQueue *queue, ssize_t qemu_net_queue_send_iov(NetQueue *queue, NetClientState *sender, - unsigned flags, const struct iovec *iov, int iovcnt, NetPacketSent *sent_cb) @@ -239,13 +230,13 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue, ssize_t ret; if (queue->delivering || !qemu_can_send_packet(sender)) { - qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb); + qemu_net_queue_append_iov(queue, sender, iov, iovcnt, sent_cb); return 0; } - ret = qemu_net_queue_deliver_iov(queue, sender, flags, iov, iovcnt); + ret = qemu_net_queue_deliver_iov(queue, sender, iov, iovcnt); if (ret == 0) { - qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb); + qemu_net_queue_append_iov(queue, sender, iov, iovcnt, sent_cb); return 0; } @@ -285,7 +276,6 @@ bool qemu_net_queue_flush(NetQueue *queue) ret = qemu_net_queue_deliver(queue, packet->sender, - packet->flags, packet->data, packet->size); if (ret == 0) { diff --git a/net/tap.c b/net/tap.c index 57389cacc3..087b2d3bc6 100644 --- a/net/tap.c +++ b/net/tap.c @@ -356,7 +356,6 @@ static NetClientInfo net_tap_info = { .type = NET_CLIENT_DRIVER_TAP, .size = sizeof(TAPState), .receive = tap_receive, - .receive_raw = tap_receive_raw, .receive_iov = tap_receive_iov, .poll = tap_poll, .cleanup = tap_cleanup,
While netmap implements virtio-net header, it does not implement receive_raw(). Instead of implementing receive_raw for netmap, add virtio-net headers in the common code and use receive_iov()/receive() instead. This also fixes the buffer size for the virtio-net header. Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- include/net/net.h | 1 - include/net/queue.h | 1 - net/net.c | 17 +++++++++-------- net/queue.c | 30 ++++++++++-------------------- net/tap.c | 1 - 5 files changed, 19 insertions(+), 31 deletions(-)