From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AFAE8C433EF for ; Tue, 15 Mar 2022 17:52:38 +0000 (UTC) Received: from localhost ([::1]:36438 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nUBLZ-0007su-LV for qemu-devel@archiver.kernel.org; Tue, 15 Mar 2022 13:52:37 -0400 Received: from eggs.gnu.org ([209.51.188.92]:43218) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nUBET-0005Ap-FH for qemu-devel@nongnu.org; Tue, 15 Mar 2022 13:45:20 -0400 Received: from [2607:f8b0:4864:20::112b] (port=43230 helo=mail-yw1-x112b.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nUBEP-0005GQ-FZ for qemu-devel@nongnu.org; Tue, 15 Mar 2022 13:45:16 -0400 Received: by mail-yw1-x112b.google.com with SMTP id 00721157ae682-2e59939b862so14712497b3.10 for ; Tue, 15 Mar 2022 10:45:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jetbrains.com; s=googleapps; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=conql31RgkP8FOERJ7yman9I05GCZ8yvZSb0rTQUKzw=; b=PMiB7GM5s+RNGQ0rRyvEgIixAaq1UKy04rd14xTd5vgcI2R3WHy3n70nxxPmbH4KiK JY58k7hDkNEpI8OxJoT4/4vjZLDuWifShIgg2R/Izv4ei9khAuViY08q3nliahwTA7qK WXZk9nWjb1C3v1HPabnc03d/8teLJku2Ae/FU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=conql31RgkP8FOERJ7yman9I05GCZ8yvZSb0rTQUKzw=; b=0PiL4z/wrcfCJ9CYzT2QWznQaLEuFL0zqEmuVoRAiWLxfOFdT5UC76gqjOPe0DX1CL ibF791jx3T7eeBQNEuqm97tiCevld31TSV7c9j8AGtGIkkf0o9EAfnL0lDVTacYJSGKd 0+x06dFW9Mo197ALFrzZ3zQrw6NivxdqRPBzeZz0Ka27q9+SoKAVW1yJd2CrQAQZK5sO 27xGrEFn1UC+aFohIQIvuZWGrmTygCxvJLKkvjopA6Jo+S9yGf6H5i6gMKqhlCfsAXRQ +6o2mRNoQX/IQkKLDO9n0W1GrgnrZVyKaJAN5CUcoIVRtQyEZWm3Ubg3ZrLjXar6RsmC 2Y6g== X-Gm-Message-State: AOAM530JmU+g3K7q/Eg1Y7Tfs798UvYXpfnyD0T0vbAG8G6yjR0tOV/Y r3eqTKPwcnkJeuFKRu8uXDNUVCVY04NFqPJjdGjrDA== X-Google-Smtp-Source: ABdhPJys6oehq1DD3Rh5sRQEXvxjkFAvIn5+JhgEy0cL7GeLf9TpUgoPxHbhWe1baJGQ4G0uBcUnB5M2hbhAaTIXkhY= X-Received: by 2002:a81:d15:0:b0:2db:d348:2fda with SMTP id 21-20020a810d15000000b002dbd3482fdamr25617012ywn.151.1647366311839; Tue, 15 Mar 2022 10:45:11 -0700 (PDT) MIME-Version: 1.0 References: <20220315100239.2914-1-Vladislav.Yaroshchuk@jetbrains.com> <20220315100239.2914-4-Vladislav.Yaroshchuk@jetbrains.com> <475cb18d-7c99-dea9-5a7b-59f72d7ed590@gmail.com> In-Reply-To: <475cb18d-7c99-dea9-5a7b-59f72d7ed590@gmail.com> From: Vladislav Yaroshchuk Date: Tue, 15 Mar 2022 20:45:01 +0300 Message-ID: Subject: Re: [PATCH v17 3/7] net/vmnet: implement shared mode (vmnet-shared) To: Akihiko Odaki Cc: qemu Developers , Jason Wang , Roman Bolshakov , Eric Blake , phillip.ennen@gmail.com, Phillip Tennen , Markus Armbruster , Howard Spoelstra , Alessio Dionisi , Roman Bolshakov , Peter Maydell , Cameron Esfahani , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Alexander Graf , Gerd Hoffmann , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Christian Schoenebeck Content-Type: multipart/alternative; boundary="000000000000d5842f05da455b27" X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::112b (failed) Received-SPF: pass client-ip=2607:f8b0:4864:20::112b; envelope-from=vladislav.yaroshchuk@jetbrains.com; helo=mail-yw1-x112b.google.com X-Spam_score_int: -6 X-Spam_score: -0.7 X-Spam_bar: / X-Spam_report: (-0.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, PDS_HP_HELO_NORDNS=0.659, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000d5842f05da455b27 Content-Type: text/plain; charset="UTF-8" On Tue, Mar 15, 2022 at 1:18 PM Akihiko Odaki wrote: > On 2022/03/15 19:02, Vladislav Yaroshchuk wrote: > > Interaction with vmnet.framework in different modes > > differs only on configuration stage, so we can create > > common `send`, `receive`, etc. procedures and reuse them. > > > > Signed-off-by: Phillip Tennen > > Signed-off-by: Vladislav Yaroshchuk > > --- > > net/vmnet-common.m | 359 +++++++++++++++++++++++++++++++++++++++++++++ > > net/vmnet-shared.c | 94 +++++++++++- > > net/vmnet_int.h | 41 +++++- > > 3 files changed, 489 insertions(+), 5 deletions(-) > > > > diff --git a/net/vmnet-common.m b/net/vmnet-common.m > > index 56612c72ce..6af042406b 100644 > > --- a/net/vmnet-common.m > > +++ b/net/vmnet-common.m > > @@ -10,6 +10,8 @@ > > */ > > > > #include "qemu/osdep.h" > > +#include "qemu/main-loop.h" > > +#include "qemu/log.h" > > #include "qapi/qapi-types-net.h" > > #include "vmnet_int.h" > > #include "clients.h" > > @@ -17,4 +19,361 @@ > > #include "qapi/error.h" > > > > #include > > +#include > > > > + > > +static void vmnet_send_completed(NetClientState *nc, ssize_t len); > > + > > + > > +const char *vmnet_status_map_str(vmnet_return_t status) > > +{ > > + switch (status) { > > + case VMNET_SUCCESS: > > + return "success"; > > + case VMNET_FAILURE: > > + return "general failure (possibly not enough privileges)"; > > + case VMNET_MEM_FAILURE: > > + return "memory allocation failure"; > > + case VMNET_INVALID_ARGUMENT: > > + return "invalid argument specified"; > > + case VMNET_SETUP_INCOMPLETE: > > + return "interface setup is not complete"; > > + case VMNET_INVALID_ACCESS: > > + return "invalid access, permission denied"; > > + case VMNET_PACKET_TOO_BIG: > > + return "packet size is larger than MTU"; > > + case VMNET_BUFFER_EXHAUSTED: > > + return "buffers exhausted in kernel"; > > + case VMNET_TOO_MANY_PACKETS: > > + return "packet count exceeds limit"; > > +#if defined(MAC_OS_VERSION_11_0) && \ > > + MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0 > > + case VMNET_SHARING_SERVICE_BUSY: > > + return "conflict, sharing service is in use"; > > +#endif > > + default: > > + return "unknown vmnet error"; > > + } > > +} > > + > > +/** > > + * Write packets from QEMU to vmnet interface. > > + * > > + * vmnet.framework supports iov, but writing more than > > + * one iov into vmnet interface fails with > > + * 'VMNET_INVALID_ARGUMENT'. Collecting provided iovs into > > + * one and passing it to vmnet works fine. That's the > > + * reason why receive_iov() left unimplemented. But it still > > + * works with good performance having .receive() only. > > + */ > > +ssize_t vmnet_receive_common(NetClientState *nc, > > + const uint8_t *buf, > > + size_t size) > > +{ > > + VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc); > > + struct vmpktdesc packet; > > + struct iovec iov; > > + int pkt_cnt; > > + vmnet_return_t if_status; > > + > > + if (size > s->max_packet_size) { > > + warn_report("vmnet: packet is too big, %zu > %" PRIu64, > > + packet.vm_pkt_size, > > + s->max_packet_size); > > + return -1; > > + } > > + > > + iov.iov_base = (char *) buf; > > + iov.iov_len = size; > > + > > + packet.vm_pkt_iovcnt = 1; > > + packet.vm_flags = 0; > > + packet.vm_pkt_size = size; > > + packet.vm_pkt_iov = &iov; > > + pkt_cnt = 1; > > + > > + if_status = vmnet_write(s->vmnet_if, &packet, &pkt_cnt); > > + if (if_status != VMNET_SUCCESS) { > > + error_report("vmnet: write error: %s\n", > > + vmnet_status_map_str(if_status)); > > + return -1; > > + } > > + > > + if (pkt_cnt) { > > + return size; > > + } > > + return 0; > > +} > > + > > +/** > > + * Read packets from vmnet interface and write them > > + * to temporary buffers in VmnetCommonState. > > + * > > + * Returns read packets number (may be 0) if read > > + * is successful, -1 on error > > + */ > > +static int vmnet_read_packets(VmnetCommonState *s) { > > + assert(s->packets_send_current_pos == s->packets_send_end_pos); > > + > > + struct vmpktdesc *packets = s->packets_buf; > > + vmnet_return_t status; > > + int i; > > + > > + /* Read as many packets as present */ > > + s->packets_send_current_pos = 0; > > + s->packets_send_end_pos = VMNET_PACKETS_LIMIT; > > + for (i = 0; i < s->packets_send_end_pos; ++i) { > > + packets[i].vm_pkt_size = s->max_packet_size; > > + packets[i].vm_pkt_iovcnt = 1; > > + packets[i].vm_flags = 0; > > + } > > + > > + status = vmnet_read(s->vmnet_if, packets, &s->packets_send_end_pos); > > + if (status != VMNET_SUCCESS) { > > + error_printf("vmnet: read failed: %s\n", > > + vmnet_status_map_str(status)); > > + s->packets_send_current_pos = 0; > > + s->packets_send_end_pos = 0; > > + return -1; > > + } > > + > > + return s->packets_send_end_pos; > > +} > > + > > +/** > > + * Write packets from temporary buffers in VmnetCommonState > > + * to QEMU. > > + */ > > +static void vmnet_write_packets_to_qemu(VmnetCommonState *s) { > > + ssize_t size; > > + > > + /* > > + * Packets to send lay in [current_pos..end_pos) > > + * (including current_pos, excluding end_pos) > > + */ > > + while (s->packets_send_current_pos < s->packets_send_end_pos) { > > + size = qemu_send_packet_async(&s->nc, > > + > s->iov_buf[s->packets_send_current_pos].iov_base, > > + > s->packets_buf[s->packets_send_current_pos].vm_pkt_size, > > + vmnet_send_completed); > > + ++s->packets_send_current_pos; > > + if (size == 0) { > > + /* QEMU is not ready to consume more packets - > > + * stop and wait for completion callback call */ > > + s->send_enabled = false; > > + return; > > + } > > + } > > + s->send_enabled = true; > > +} > > + > > + > > +/** > > + * Bottom half callback that transfers packets from vmnet interface > > + * to QEMU. > > + * > > + * The process of transferring packets is three-staged: > > + * 1. Handle vmnet event; > > + * 2. Read packets from vmnet interface into temporary buffer; > > + * 3. Write packets from temporary buffer to QEMU. > > + * > > + * QEMU may suspend this process on the last stage, returning 0 from > > + * qemu_send_packet_async function. If this happens, we should > > + * respectfully wait until it is ready to consume more packets, > > + * write left ones in temporary buffer and only after this > > + * continue reading more packets from vmnet interface. > > + * > > + * If QEMU is not ready, send_enabled is set to false. > > + * > > + * Packets to be transferred are stored into packets_buf, > > + * in the window [packets_send_current_pos..packets_send_end_pos) > > + * including current_pos, excluding end_pos. > > + */ > > +static void vmnet_send_bh(void *opaque) > > +{ > > + NetClientState *nc = (NetClientState *) opaque; > > + VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc); > > + > > + /* > > + * Do nothing if QEMU is not ready - wait > > + * for completion callback invocation > > + */ > > + if (!s->send_enabled) { > > + return; > > + } > > + > > + /* Read packets from vmnet interface */ > > + if (vmnet_read_packets(s) > 0) { > > + /* Send them to QEMU */ > > + vmnet_write_packets_to_qemu(s); > > + } > > +} > > + > > +/** > > + * Completion callback to be invoked by QEMU when it becomes > > + * ready to consume more packets. > > + */ > > +static void vmnet_send_completed(NetClientState *nc, ssize_t len) > > +{ > > + VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc); > > + > > + /* Complete sending packets left in VmnetCommonState buffers */ > > + vmnet_write_packets_to_qemu(s); > > + > > + /* And read new ones from vmnet interface */ > > + if (s->send_enabled) { > > + qemu_bh_schedule(s->send_bh); > > + } > > +} > > + > > +static void vmnet_bufs_init(VmnetCommonState *s) > > +{ > > + struct vmpktdesc *packets = s->packets_buf; > > + struct iovec *iov = s->iov_buf; > > + int i; > > + > > + for (i = 0; i < VMNET_PACKETS_LIMIT; ++i) { > > + iov[i].iov_len = s->max_packet_size; > > + iov[i].iov_base = g_malloc0(iov[i].iov_len); > > + packets[i].vm_pkt_iov = iov + i; > > + } > > +} > > + > > + > > +int vmnet_if_create(NetClientState *nc, > > + xpc_object_t if_desc, > > + Error **errp) > > +{ > > + VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc); > > + dispatch_semaphore_t if_created_sem = dispatch_semaphore_create(0); > > + __block vmnet_return_t if_status; > > + > > + s->if_queue = dispatch_queue_create( > > + "org.qemu.vmnet.if_queue", > > + DISPATCH_QUEUE_SERIAL > > + ); > > + > > + xpc_dictionary_set_bool( > > + if_desc, > > + vmnet_allocate_mac_address_key, > > + false > > + ); > > + > > +#ifdef DEBUG > > + qemu_log("vmnet.start.interface_desc:\n"); > > + xpc_dictionary_apply(if_desc, > > + ^bool(const char *k, xpc_object_t v) { > > + char *desc = xpc_copy_description(v); > > + qemu_log(" %s=%s\n", k, desc); > > + free(desc); > > + return true; > > + }); > > +#endif /* DEBUG */ > > + > > + s->vmnet_if = vmnet_start_interface( > > + if_desc, > > + s->if_queue, > > + ^(vmnet_return_t status, xpc_object_t interface_param) { > > + if_status = status; > > + if (status != VMNET_SUCCESS || !interface_param) { > > + dispatch_semaphore_signal(if_created_sem); > > + return; > > + } > > + > > +#ifdef DEBUG > > + qemu_log("vmnet.start.interface_param:\n"); > > + xpc_dictionary_apply(interface_param, > > + ^bool(const char *k, xpc_object_t v) { > > + char *desc = > xpc_copy_description(v); > > + qemu_log(" %s=%s\n", k, desc); > > + free(desc); > > + return true; > > + }); > > +#endif /* DEBUG */ > > + > > + s->mtu = xpc_dictionary_get_uint64( > > + interface_param, > > + vmnet_mtu_key); > > + s->max_packet_size = xpc_dictionary_get_uint64( > > + interface_param, > > + vmnet_max_packet_size_key); > > + > > + dispatch_semaphore_signal(if_created_sem); > > + }); > > + > > + if (s->vmnet_if == NULL) { > > + dispatch_release(s->if_queue); > > + dispatch_release(if_created_sem); > > + error_setg(errp, > > + "unable to create interface with requested params"); > > + return -1; > > + } > > + > > + dispatch_semaphore_wait(if_created_sem, DISPATCH_TIME_FOREVER); > > + dispatch_release(if_created_sem); > > + > > + if (if_status != VMNET_SUCCESS) { > > + dispatch_release(s->if_queue); > > + error_setg(errp, > > + "cannot create vmnet interface: %s", > > + vmnet_status_map_str(if_status)); > > + return -1; > > + } > > + > > + s->send_bh = aio_bh_new(qemu_get_aio_context(), vmnet_send_bh, nc); > > + s->send_enabled = true; > > + vmnet_bufs_init(s); > > + > > + vmnet_interface_set_event_callback( > > + s->vmnet_if, > > + VMNET_INTERFACE_PACKETS_AVAILABLE, > > + s->if_queue, > > + ^(interface_event_t event_id, xpc_object_t event) { > > + assert(event_id == VMNET_INTERFACE_PACKETS_AVAILABLE); > > + /* > > + * This function is being called from a non qemu thread, so > > + * we only schedule a BH, and do the rest of the io > completion > > + * handling from vmnet_send_bh() which runs in a qemu > context. > > + */ > > + qemu_bh_schedule(s->send_bh); > > + }); > > + > > + return 0; > > +} > > + > > + > > +void vmnet_cleanup_common(NetClientState *nc) > > +{ > > + VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc); > > + dispatch_semaphore_t if_stopped_sem; > > + > > + if (s->vmnet_if == NULL) { > > + return; > > + } > > + > > + vmnet_interface_set_event_callback( > > + s->vmnet_if, > > + VMNET_INTERFACE_PACKETS_AVAILABLE, > > + NULL, > > + NULL); > > As I stated in an earlier comment, it should not call > vmnet_interface_set_event_callback and instead let vmnet.framework > decide the proper order of deinitialization as it should know the > internals. If you are concerned with the case it receives packets while > calling qemu_purge_queued_packets(nc), the call can be moved after > vmnet_stop_interface. > > Ok, I will update this part. > > + > > + qemu_purge_queued_packets(nc); > > + > > + if_stopped_sem = dispatch_semaphore_create(0); > > + vmnet_stop_interface( > > + s->vmnet_if, > > + s->if_queue, > > + ^(vmnet_return_t status) { > > + assert(status == VMNET_SUCCESS); > > + dispatch_semaphore_signal(if_stopped_sem); > > + }); > > + dispatch_semaphore_wait(if_stopped_sem, DISPATCH_TIME_FOREVER); > > + > > + qemu_bh_delete(s->send_bh); > > + dispatch_release(if_stopped_sem); > > + dispatch_release(s->if_queue); > > + > > + for (int i = 0; i < VMNET_PACKETS_LIMIT; ++i) { > > + g_free(s->iov_buf[i].iov_base); > > + } > > +} > > diff --git a/net/vmnet-shared.c b/net/vmnet-shared.c > > index f07afaaf21..e1a7e30acb 100644 > > --- a/net/vmnet-shared.c > > +++ b/net/vmnet-shared.c > > @@ -10,16 +10,102 @@ > > > > #include "qemu/osdep.h" > > #include "qapi/qapi-types-net.h" > > +#include "qapi/error.h" > > #include "vmnet_int.h" > > #include "clients.h" > > -#include "qemu/error-report.h" > > -#include "qapi/error.h" > > > > #include > > > > +typedef struct VmnetSharedState { > > + VmnetCommonState cs; > > +} VmnetSharedState; > > + > > + > > +static bool validate_options(const Netdev *netdev, Error **errp) > > +{ > > + const NetdevVmnetSharedOptions *options = &(netdev->u.vmnet_shared); > > + > > +#if !defined(MAC_OS_VERSION_11_0) || \ > > + MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_11_0 > > + if (options->has_isolated) { > > + error_setg(errp, > > + "vmnet-shared.isolated feature is " > > + "unavailable: outdated vmnet.framework API"); > > + return false; > > + } > > +#endif > > + > > + if ((options->has_start_address || > > + options->has_end_address || > > + options->has_subnet_mask) && > > + !(options->has_start_address && > > + options->has_end_address && > > + options->has_subnet_mask)) { > > + error_setg(errp, > > + "'start-address', 'end-address', 'subnet-mask' " > > + "should be provided together" > > + ); > > + return false; > > + } > > + > > + return true; > > +} > > + > > +static xpc_object_t build_if_desc(const Netdev *netdev) > > +{ > > + const NetdevVmnetSharedOptions *options = &(netdev->u.vmnet_shared); > > + xpc_object_t if_desc = xpc_dictionary_create(NULL, NULL, 0); > > + > > + xpc_dictionary_set_uint64( > > + if_desc, > > + vmnet_operation_mode_key, > > + VMNET_SHARED_MODE > > + ); > > + > > + if (options->has_nat66_prefix) { > > + xpc_dictionary_set_string(if_desc, > > + vmnet_nat66_prefix_key, > > + options->nat66_prefix); > > + } > > + > > + if (options->has_start_address) { > > + xpc_dictionary_set_string(if_desc, > > + vmnet_start_address_key, > > + options->start_address); > > + xpc_dictionary_set_string(if_desc, > > + vmnet_end_address_key, > > + options->end_address); > > + xpc_dictionary_set_string(if_desc, > > + vmnet_subnet_mask_key, > > + options->subnet_mask); > > + } > > + > > +#if defined(MAC_OS_VERSION_11_0) && \ > > + MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0 > > + xpc_dictionary_set_bool( > > + if_desc, > > + vmnet_enable_isolation_key, > > + options->isolated > > + ); > > +#endif > > + > > + return if_desc; > > +} > > + > > +static NetClientInfo net_vmnet_shared_info = { > > + .type = NET_CLIENT_DRIVER_VMNET_SHARED, > > + .size = sizeof(VmnetSharedState), > > + .receive = vmnet_receive_common, > > + .cleanup = vmnet_cleanup_common, > > +}; > > + > > int net_init_vmnet_shared(const Netdev *netdev, const char *name, > > NetClientState *peer, Error **errp) > > { > > - error_setg(errp, "vmnet-shared is not implemented yet"); > > - return -1; > > + NetClientState *nc = qemu_new_net_client(&net_vmnet_shared_info, > > + peer, "vmnet-shared", > name); > > + if (!validate_options(netdev, errp)) { > > + return -1; > > + } > > + return vmnet_if_create(nc, build_if_desc(netdev), errp); > > } > > diff --git a/net/vmnet_int.h b/net/vmnet_int.h > > index aac4d5af64..8f3321ef3e 100644 > > --- a/net/vmnet_int.h > > +++ b/net/vmnet_int.h > > @@ -15,11 +15,50 @@ > > #include "clients.h" > > > > #include > > +#include > > + > > +/** > > + * From vmnet.framework documentation > > + * > > + * Each read/write call allows up to 200 packets to be > > + * read or written for a maximum of 256KB. > > + * > > + * Each packet written should be a complete > > + * ethernet frame. > > + * > > + * https://developer.apple.com/documentation/vmnet > > + */ > > +#define VMNET_PACKETS_LIMIT 200 > > > > typedef struct VmnetCommonState { > > - NetClientState nc; > > + NetClientState nc; > > + interface_ref vmnet_if; > > + > > + uint64_t mtu; > > + uint64_t max_packet_size; > > > > + dispatch_queue_t if_queue; > > + > > + QEMUBH *send_bh; > > + bool send_enabled; > > I'm still not convinced it is preferred to have send_enabled and > packets_send_current_pos not to point to the packet currently being sent > when qemu_send_packet_async returns 0 either. > By incrementing packets_send_current_pos in vmnet_send_completed instead > of immediately after calling qemu_send_packet_async, it can always > represent the position of the packet currently being sent. It would also > allow to directly state the condition to enable sending in vmnet_send_bh > without involving indirection with send_enabled. > > > incrementing packets_send_current_pos in vmnet_send_completed It is a wrong idea I think. QEMU calls callback only if previously qemu_send_packet_async returned 0. If the packet was sent immediately (returned value > 0), the callback won't be invoked. If not (0 is returned), the callback is expected to be called when QEMU is ready. So, the callback is not about "packet sent", but it is about "QEMU is ready now" A simple proof with some debug printing: static void vmnet_write_packets_to_qemu(VmnetState *s) { ssize_t size; while (s->packets_send_current_pos < s->packets_send_end_pos) { size = qemu_send_packet_async(&s->nc, s->iov_buf[s->packets_send_current_pos].iov_base, s->packets_buf[s->packets_send_current_pos].vm_pkt_size, vmnet_send_completed); ++s->packets_send_current_pos; if (size == 0) { fprintf(stderr, "stop\n"); // (!) s->send_enabled = false; return; } fprintf(stderr, "ok\n"); // (!) } s->send_enabled = true; } static void vmnet_send_completed(NetClientState *nc, ssize_t len) { fprintf(stderr, "continue\n"); // (!) VmnetState *s = DO_UPCAST(VmnetState, nc, nc); vmnet_write_packets_to_qemu(s); if (s->send_enabled) { qemu_bh_schedule(s->send_bh); } } And the result (while both iperf3 + ping flood): ok ok ok ok ok ok ok stop continue ok ok ok stop continue stop continue stop continue ok ok ok stop continue As you can see, if the packet is sent immediately (ok), the callback is not invoked. If 0 is returned (stop), the callback is invoked (continue) when QEMU is ready. Taking this into account, I don't see any way to get rid of the send_enabled flag without making implementation more complex. Best regards, Vladislav Yaroshchuk Regards, > Akihiko Odaki > > > + > > + struct vmpktdesc packets_buf[VMNET_PACKETS_LIMIT]; > > + int packets_send_current_pos; > > + int packets_send_end_pos; > > + > > + struct iovec iov_buf[VMNET_PACKETS_LIMIT]; > > } VmnetCommonState; > > > > +const char *vmnet_status_map_str(vmnet_return_t status); > > + > > +int vmnet_if_create(NetClientState *nc, > > + xpc_object_t if_desc, > > + Error **errp); > > + > > +ssize_t vmnet_receive_common(NetClientState *nc, > > + const uint8_t *buf, > > + size_t size); > > + > > +void vmnet_cleanup_common(NetClientState *nc); > > > > #endif /* VMNET_INT_H */ > > --000000000000d5842f05da455b27 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Mar 15, 2022 at 1:18 PM Akihi= ko Odaki <a= kihiko.odaki@gmail.com> wrote:
On 2022/03/15 19:02, Vladislav Yaroshchuk wrote:
> Interaction with vmnet.framework in different modes
> differs only on configuration stage, so we can create
> common `send`, `receive`, etc. procedures and reuse them.
>
> Signed-off-by: Phillip Tennen <phillip@axleos.com>
> Signed-off-by: Vladislav Yaroshchuk <Vladislav.Yaroshchuk@jetbrains.co= m>
> ---
>=C2=A0 =C2=A0net/vmnet-common.m | 359 +++++++++++++++++++++++++++++++++= ++++++++++++
>=C2=A0 =C2=A0net/vmnet-shared.c |=C2=A0 94 +++++++++++-
>=C2=A0 =C2=A0net/vmnet_int.h=C2=A0 =C2=A0 |=C2=A0 41 +++++-
>=C2=A0 =C2=A03 files changed, 489 insertions(+), 5 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 56612c72ce..6af042406b 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -10,6 +10,8 @@
>=C2=A0 =C2=A0 */
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/log.h"
>=C2=A0 =C2=A0#include "qapi/qapi-types-net.h"
>=C2=A0 =C2=A0#include "vmnet_int.h"
>=C2=A0 =C2=A0#include "clients.h"
> @@ -17,4 +19,361 @@
>=C2=A0 =C2=A0#include "qapi/error.h"
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#include <vmnet/vmnet.h>
> +#include <dispatch/dispatch.h>
>=C2=A0 =C2=A0
> +
> +static void vmnet_send_completed(NetClientState *nc, ssize_t len); > +
> +
> +const char *vmnet_status_map_str(vmnet_return_t status)
> +{
> +=C2=A0 =C2=A0 switch (status) {
> +=C2=A0 =C2=A0 case VMNET_SUCCESS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "success";
> +=C2=A0 =C2=A0 case VMNET_FAILURE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "general failure (possibly no= t enough privileges)";
> +=C2=A0 =C2=A0 case VMNET_MEM_FAILURE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "memory allocation failure&qu= ot;;
> +=C2=A0 =C2=A0 case VMNET_INVALID_ARGUMENT:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "invalid argument specified&q= uot;;
> +=C2=A0 =C2=A0 case VMNET_SETUP_INCOMPLETE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "interface setup is not compl= ete";
> +=C2=A0 =C2=A0 case VMNET_INVALID_ACCESS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "invalid access, permission d= enied";
> +=C2=A0 =C2=A0 case VMNET_PACKET_TOO_BIG:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "packet size is larger than M= TU";
> +=C2=A0 =C2=A0 case VMNET_BUFFER_EXHAUSTED:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "buffers exhausted in kernel&= quot;;
> +=C2=A0 =C2=A0 case VMNET_TOO_MANY_PACKETS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "packet count exceeds limit&q= uot;;
> +#if defined(MAC_OS_VERSION_11_0) && \
> +=C2=A0 =C2=A0 MAC_OS_X_VERSION_MIN_REQUIRED >=3D MAC_OS_VERSION_11= _0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 case VMNET_SHARING_SERVICE_BUSY:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "conflict, sharing service is= in use";
> +#endif
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "unknown vmnet error"; > +=C2=A0 =C2=A0 }
> +}
> +
> +/**
> + * Write packets from QEMU to vmnet interface.
> + *
> + * vmnet.framework supports iov, but writing more than
> + * one iov into vmnet interface fails with
> + * 'VMNET_INVALID_ARGUMENT'. Collecting provided iovs into > + * one and passing it to vmnet works fine. That's the
> + * reason why receive_iov() left unimplemented. But it still
> + * works with good performance having .receive() only.
> + */
> +ssize_t vmnet_receive_common(NetClientState *nc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0const uint8_t *buf,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size_t size)
> +{
> +=C2=A0 =C2=A0 VmnetCommonState *s =3D DO_UPCAST(VmnetCommonState, nc,= nc);
> +=C2=A0 =C2=A0 struct vmpktdesc packet;
> +=C2=A0 =C2=A0 struct iovec iov;
> +=C2=A0 =C2=A0 int pkt_cnt;
> +=C2=A0 =C2=A0 vmnet_return_t if_status;
> +
> +=C2=A0 =C2=A0 if (size > s->max_packet_size) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 warn_report("vmnet: packet is too bi= g, %zu > %" PRIu64,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 packet.vm_pkt_size,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->max_packet_size);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 iov.iov_base =3D (char *) buf;
> +=C2=A0 =C2=A0 iov.iov_len =3D size;
> +
> +=C2=A0 =C2=A0 packet.vm_pkt_iovcnt =3D 1;
> +=C2=A0 =C2=A0 packet.vm_flags =3D 0;
> +=C2=A0 =C2=A0 packet.vm_pkt_size =3D size;
> +=C2=A0 =C2=A0 packet.vm_pkt_iov =3D &iov;
> +=C2=A0 =C2=A0 pkt_cnt =3D 1;
> +
> +=C2=A0 =C2=A0 if_status =3D vmnet_write(s->vmnet_if, &packet, = &pkt_cnt);
> +=C2=A0 =C2=A0 if (if_status !=3D VMNET_SUCCESS) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_report("vmnet: write error: %s= \n",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0vmnet_status_map_str(if_status));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (pkt_cnt) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return size;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 return 0;
> +}
> +
> +/**
> + * Read packets from vmnet interface and write them
> + * to temporary buffers in VmnetCommonState.
> + *
> + * Returns read packets number (may be 0) if read
> + * is successful, -1 on error
> + */
> +static int vmnet_read_packets(VmnetCommonState *s) {
> +=C2=A0 =C2=A0 assert(s->packets_send_current_pos =3D=3D s->pack= ets_send_end_pos);
> +
> +=C2=A0 =C2=A0 struct vmpktdesc *packets =3D s->packets_buf;
> +=C2=A0 =C2=A0 vmnet_return_t status;
> +=C2=A0 =C2=A0 int i;
> +
> +=C2=A0 =C2=A0 /* Read as many packets as present */
> +=C2=A0 =C2=A0 s->packets_send_current_pos =3D 0;
> +=C2=A0 =C2=A0 s->packets_send_end_pos =3D VMNET_PACKETS_LIMIT;
> +=C2=A0 =C2=A0 for (i =3D 0; i < s->packets_send_end_pos; ++i) {=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 packets[i].vm_pkt_size =3D s->max_pack= et_size;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 packets[i].vm_pkt_iovcnt =3D 1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 packets[i].vm_flags =3D 0;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 status =3D vmnet_read(s->vmnet_if, packets, &s-&= gt;packets_send_end_pos);
> +=C2=A0 =C2=A0 if (status !=3D VMNET_SUCCESS) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_printf("vmnet: read failed: %s= \n",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0vmnet_status_map_str(status));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->packets_send_current_pos =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->packets_send_end_pos =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return s->packets_send_end_pos;
> +}
> +
> +/**
> + * Write packets from temporary buffers in VmnetCommonState
> + * to QEMU.
> + */
> +static void vmnet_write_packets_to_qemu(VmnetCommonState *s) {
> +=C2=A0 =C2=A0 ssize_t size;
> +
> +=C2=A0 =C2=A0 /*
> +=C2=A0 =C2=A0 =C2=A0* Packets to send lay in [current_pos..end_pos) > +=C2=A0 =C2=A0 =C2=A0* (including current_pos, excluding end_pos)
> +=C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 while (s->packets_send_current_pos < s->packet= s_send_end_pos) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 size =3D qemu_send_packet_async(&s-&g= t;nc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->iov_b= uf[s->packets_send_current_pos].iov_base,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->packe= ts_buf[s->packets_send_current_pos].vm_pkt_size,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_send_= completed);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ++s->packets_send_current_pos;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (size =3D=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* QEMU is not ready to con= sume more packets -
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* stop and wait for c= ompletion callback call */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->send_enabled =3D fals= e;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 s->send_enabled =3D true;
> +}
> +
> +
> +/**
> + * Bottom half callback that transfers packets from vmnet interface > + * to QEMU.
> + *
> + * The process of transferring packets is three-staged:
> + * 1. Handle vmnet event;
> + * 2. Read packets from vmnet interface into temporary buffer;
> + * 3. Write packets from temporary buffer to QEMU.
> + *
> + * QEMU may suspend this process on the last stage, returning 0 from<= br> > + * qemu_send_packet_async function. If this happens, we should
> + * respectfully wait until it is ready to consume more packets,
> + * write left ones in temporary buffer and only after this
> + * continue reading more packets from vmnet interface.
> + *
> + * If QEMU is not ready, send_enabled is set to false.
> + *
> + * Packets to be transferred are stored into packets_buf,
> + * in the window [packets_send_current_pos..packets_send_end_pos)
> + * including current_pos, excluding end_pos.
> + */
> +static void vmnet_send_bh(void *opaque)
> +{
> +=C2=A0 =C2=A0 NetClientState *nc =3D (NetClientState *) opaque;
> +=C2=A0 =C2=A0 VmnetCommonState *s =3D DO_UPCAST(VmnetCommonState, nc,= nc);
> +
> +=C2=A0 =C2=A0 /*
> +=C2=A0 =C2=A0 =C2=A0* Do nothing if QEMU is not ready - wait
> +=C2=A0 =C2=A0 =C2=A0* for completion callback invocation
> +=C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 if (!s->send_enabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 /* Read packets from vmnet interface */
> +=C2=A0 =C2=A0 if (vmnet_read_packets(s) > 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Send them to QEMU */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_write_packets_to_qemu(s);
> +=C2=A0 =C2=A0 }
> +}
> +
> +/**
> + * Completion callback to be invoked by QEMU when it becomes
> + * ready to consume more packets.
> + */
> +static void vmnet_send_completed(NetClientState *nc, ssize_t len)
> +{
> +=C2=A0 =C2=A0 VmnetCommonState *s =3D DO_UPCAST(VmnetCommonState, nc,= nc);
> +
> +=C2=A0 =C2=A0 /* Complete sending packets left in VmnetCommonState bu= ffers */
> +=C2=A0 =C2=A0 vmnet_write_packets_to_qemu(s);
> +
> +=C2=A0 =C2=A0 /* And read new ones from vmnet interface */
> +=C2=A0 =C2=A0 if (s->send_enabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_bh_schedule(s->send_bh);
> +=C2=A0 =C2=A0 }
> +}
> +
> +static void vmnet_bufs_init(VmnetCommonState *s)
> +{
> +=C2=A0 =C2=A0 struct vmpktdesc *packets =3D s->packets_buf;
> +=C2=A0 =C2=A0 struct iovec *iov =3D s->iov_buf;
> +=C2=A0 =C2=A0 int i;
> +
> +=C2=A0 =C2=A0 for (i =3D 0; i < VMNET_PACKETS_LIMIT; ++i) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 iov[i].iov_len =3D s->max_packet_size;=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 iov[i].iov_base =3D g_malloc0(iov[i].iov_= len);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 packets[i].vm_pkt_iov =3D iov + i;
> +=C2=A0 =C2=A0 }
> +}
> +
> +
> +int vmnet_if_create(NetClientState *nc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= xpc_object_t if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= Error **errp)
> +{
> +=C2=A0 =C2=A0 VmnetCommonState *s =3D DO_UPCAST(VmnetCommonState, nc,= nc);
> +=C2=A0 =C2=A0 dispatch_semaphore_t if_created_sem =3D dispatch_semaph= ore_create(0);
> +=C2=A0 =C2=A0 __block vmnet_return_t if_status;
> +
> +=C2=A0 =C2=A0 s->if_queue =3D dispatch_queue_create(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 "org.qemu.vmnet.if_queue",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 DISPATCH_QUEUE_SERIAL
> +=C2=A0 =C2=A0 );
> +
> +=C2=A0 =C2=A0 xpc_dictionary_set_bool(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_allocate_mac_address_key,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 false
> +=C2=A0 =C2=A0 );
> +
> +#ifdef DEBUG
> +=C2=A0 =C2=A0 qemu_log("vmnet.start.interface_desc:\n"); > +=C2=A0 =C2=A0 xpc_dictionary_apply(if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0^bool(const char *k, xpc_object_t v) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char *desc =3D xpc_copy_description(v);<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_log("=C2=A0 %s=3D%s\n", k= , desc);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0free(desc);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return true;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0});
> +#endif /* DEBUG */
> +
> +=C2=A0 =C2=A0 s->vmnet_if =3D vmnet_start_interface(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->if_queue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^(vmnet_return_t status, xpc_object_t int= erface_param) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if_status =3D status;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (status !=3D VMNET_SUCCE= SS || !interface_param) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_sema= phore_signal(if_created_sem);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +
> +#ifdef DEBUG
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_log("vmnet.start.= interface_param:\n");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xpc_dictionary_apply(interf= ace_param,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^bool(const char *k, xpc_o= bject_t v) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char *desc = =3D xpc_copy_description(v);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_log(&qu= ot;=C2=A0 %s=3D%s\n", k, desc);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0free(desc);<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return true;=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0});
> +#endif /* DEBUG */
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->mtu =3D xpc_dictionar= y_get_uint64(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 interface_par= am,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_mtu_key= );
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->max_packet_size =3D x= pc_dictionary_get_uint64(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 interface_par= am,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_max_pac= ket_size_key);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_semaphore_signal(i= f_created_sem);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 });
> +
> +=C2=A0 =C2=A0 if (s->vmnet_if =3D=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_release(s->if_queue);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_release(if_created_sem);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_setg(errp,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "unable to create interface with requested params");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 dispatch_semaphore_wait(if_created_sem, DISPATCH_TIME_F= OREVER);
> +=C2=A0 =C2=A0 dispatch_release(if_created_sem);
> +
> +=C2=A0 =C2=A0 if (if_status !=3D VMNET_SUCCESS) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_release(s->if_queue);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_setg(errp,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "cannot create vmnet interface: %s",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= vmnet_status_map_str(if_status));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 s->send_bh =3D aio_bh_new(qemu_get_aio_context(), vm= net_send_bh, nc);
> +=C2=A0 =C2=A0 s->send_enabled =3D true;
> +=C2=A0 =C2=A0 vmnet_bufs_init(s);
> +
> +=C2=A0 =C2=A0 vmnet_interface_set_event_callback(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->vmnet_if,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMNET_INTERFACE_PACKETS_AVAILABLE,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->if_queue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^(interface_event_t event_id, xpc_object_= t event) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 assert(event_id =3D=3D VMNE= T_INTERFACE_PACKETS_AVAILABLE);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* This function is be= ing called from a non qemu thread, so
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* we only schedule a = BH, and do the rest of the io completion
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* handling from vmnet= _send_bh() which runs in a qemu context.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_bh_schedule(s->send= _bh);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 });
> +
> +=C2=A0 =C2=A0 return 0;
> +}
> +
> +
> +void vmnet_cleanup_common(NetClientState *nc)
> +{
> +=C2=A0 =C2=A0 VmnetCommonState *s =3D DO_UPCAST(VmnetCommonState, nc,= nc);
> +=C2=A0 =C2=A0 dispatch_semaphore_t if_stopped_sem;
> +
> +=C2=A0 =C2=A0 if (s->vmnet_if =3D=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 vmnet_interface_set_event_callback(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->vmnet_if,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMNET_INTERFACE_PACKETS_AVAILABLE,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL);

As I stated in an earlier comment, it should not call
vmnet_interface_set_event_callback and instead let vmnet.framework
decide the proper order of deinitialization as it should know the
internals. If you are concerned with the case it receives packets while calling qemu_purge_queued_packets(nc), the call can be moved after
vmnet_stop_interface.


Ok, I will update this part.
=C2=A0
> +
> +=C2=A0 =C2=A0 qemu_purge_queued_packets(nc);
> +
> +=C2=A0 =C2=A0 if_stopped_sem =3D dispatch_semaphore_create(0);
> +=C2=A0 =C2=A0 vmnet_stop_interface(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->vmnet_if,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->if_queue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^(vmnet_return_t status) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 assert(status =3D=3D VMNET_= SUCCESS);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dispatch_semaphore_signal(i= f_stopped_sem);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 });
> +=C2=A0 =C2=A0 dispatch_semaphore_wait(if_stopped_sem, DISPATCH_TIME_F= OREVER);
> +
> +=C2=A0 =C2=A0 qemu_bh_delete(s->send_bh);
> +=C2=A0 =C2=A0 dispatch_release(if_stopped_sem);
> +=C2=A0 =C2=A0 dispatch_release(s->if_queue);
> +
> +=C2=A0 =C2=A0 for (int i =3D 0; i < VMNET_PACKETS_LIMIT; ++i) { > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 g_free(s->iov_buf[i].iov_base);
> +=C2=A0 =C2=A0 }
> +}
> diff --git a/net/vmnet-shared.c b/net/vmnet-shared.c
> index f07afaaf21..e1a7e30acb 100644
> --- a/net/vmnet-shared.c
> +++ b/net/vmnet-shared.c
> @@ -10,16 +10,102 @@
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#include "qemu/osdep.h"
>=C2=A0 =C2=A0#include "qapi/qapi-types-net.h"
> +#include "qapi/error.h"
>=C2=A0 =C2=A0#include "vmnet_int.h"
>=C2=A0 =C2=A0#include "clients.h"
> -#include "qemu/error-report.h"
> -#include "qapi/error.h"
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#include <vmnet/vmnet.h>
>=C2=A0 =C2=A0
> +typedef struct VmnetSharedState {
> +=C2=A0 =C2=A0 VmnetCommonState cs;
> +} VmnetSharedState;
> +
> +
> +static bool validate_options(const Netdev *netdev, Error **errp)
> +{
> +=C2=A0 =C2=A0 const NetdevVmnetSharedOptions *options =3D &(netde= v->u.vmnet_shared);
> +
> +#if !defined(MAC_OS_VERSION_11_0) || \
> +=C2=A0 =C2=A0 MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_11_0<= br> > +=C2=A0 =C2=A0 if (options->has_isolated) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_setg(errp,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "vmnet-shared.isolated feature is "
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "unavailable: outdated vmnet.framework API");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +#endif
> +
> +=C2=A0 =C2=A0 if ((options->has_start_address ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0options->has_end_address ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0options->has_subnet_mask) &&= amp;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 !(options->has_start_address &&= ;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->has_end_address &&= amp;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->has_subnet_mask)) { > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_setg(errp,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "'start-address', 'end-address', 'subnet-mask'= "
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= "should be provided together"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 );
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static xpc_object_t build_if_desc(const Netdev *netdev)
> +{
> +=C2=A0 =C2=A0 const NetdevVmnetSharedOptions *options =3D &(netde= v->u.vmnet_shared);
> +=C2=A0 =C2=A0 xpc_object_t if_desc =3D xpc_dictionary_create(NULL, NU= LL, 0);
> +
> +=C2=A0 =C2=A0 xpc_dictionary_set_uint64(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_operation_mode_key,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMNET_SHARED_MODE
> +=C2=A0 =C2=A0 );
> +
> +=C2=A0 =C2=A0 if (options->has_nat66_prefix) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 xpc_dictionary_set_string(if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_nat66_prefix_key, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->nat66_prefix)= ;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (options->has_start_address) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 xpc_dictionary_set_string(if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_start_address_key,<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->start_address= );
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 xpc_dictionary_set_string(if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_end_address_key, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->end_address);=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 xpc_dictionary_set_string(if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_subnet_mask_key, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 options->subnet_mask);=
> +=C2=A0 =C2=A0 }
> +
> +#if defined(MAC_OS_VERSION_11_0) && \
> +=C2=A0 =C2=A0 MAC_OS_X_VERSION_MIN_REQUIRED >=3D MAC_OS_VERSION_11= _0
> +=C2=A0 =C2=A0 xpc_dictionary_set_bool(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_enable_isolation_key,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 options->isolated
> +=C2=A0 =C2=A0 );
> +#endif
> +
> +=C2=A0 =C2=A0 return if_desc;
> +}
> +
> +static NetClientInfo net_vmnet_shared_info =3D {
> +=C2=A0 =C2=A0 .type =3D NET_CLIENT_DRIVER_VMNET_SHARED,
> +=C2=A0 =C2=A0 .size =3D sizeof(VmnetSharedState),
> +=C2=A0 =C2=A0 .receive =3D vmnet_receive_common,
> +=C2=A0 =C2=A0 .cleanup =3D vmnet_cleanup_common,
> +};
> +
>=C2=A0 =C2=A0int net_init_vmnet_shared(const Netdev *netdev, const char= *name,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0NetClientState *peer, Error **errp)
>=C2=A0 =C2=A0{
> -=C2=A0 error_setg(errp, "vmnet-shared is not implemented yet&quo= t;);
> -=C2=A0 return -1;
> +=C2=A0 =C2=A0 NetClientState *nc =3D qemu_new_net_client(&net_vmn= et_shared_info,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0peer, "vmnet-shared", name);
> +=C2=A0 =C2=A0 if (!validate_options(netdev, errp)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 return vmnet_if_create(nc, build_if_desc(netdev), errp)= ;
>=C2=A0 =C2=A0}
> diff --git a/net/vmnet_int.h b/net/vmnet_int.h
> index aac4d5af64..8f3321ef3e 100644
> --- a/net/vmnet_int.h
> +++ b/net/vmnet_int.h
> @@ -15,11 +15,50 @@
>=C2=A0 =C2=A0#include "clients.h"
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#include <vmnet/vmnet.h>
> +#include <dispatch/dispatch.h>
> +
> +/**
> + *=C2=A0 From vmnet.framework documentation
> + *
> + *=C2=A0 Each read/write call allows up to 200 packets to be
> + *=C2=A0 read or written for a maximum of 256KB.
> + *
> + *=C2=A0 Each packet written should be a complete
> + *=C2=A0 ethernet frame.
> + *
> + *=C2=A0 https://developer.apple.com/documentat= ion/vmnet
> + */
> +#define VMNET_PACKETS_LIMIT 200
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0typedef struct VmnetCommonState {
> -=C2=A0 NetClientState nc;
> +=C2=A0 =C2=A0 NetClientState nc;
> +=C2=A0 =C2=A0 interface_ref vmnet_if;
> +
> +=C2=A0 =C2=A0 uint64_t mtu;
> +=C2=A0 =C2=A0 uint64_t max_packet_size;
>=C2=A0 =C2=A0
> +=C2=A0 =C2=A0 dispatch_queue_t if_queue;
> +
> +=C2=A0 =C2=A0 QEMUBH *send_bh;
> +=C2=A0 =C2=A0 bool send_enabled;

I'm still not convinced it is preferred to have send_enabled and
packets_send_current_pos not to point to the packet currently being sent when qemu_send_packet_async returns 0 either.
By incrementing packets_send_current_pos in vmnet_send_completed instead of immediately after calling qemu_send_packet_async, it can always
represent the position of the packet currently being sent. It would also allow to directly state the condition to enable sending in vmnet_send_bh without involving indirection with send_enabled.


> incrementing packets_send_current= _pos in vmnet_send_completed

It is a wrong idea I think. = QEMU calls callback only if previously
qemu_send_packet_async returned 0= . If the packet was sent
immediately=C2=A0(returned value > 0), the c= allback won't be invoked.
If not (0 is returned), the callbac= k is expected to be called when
QEMU is ready. So, the callback is not a= bout "packet sent", but it is=C2=A0
about "QEMU is= ready now"

A simple proof with some debug printing:

sta= tic void vmnet_write_packets_to_qemu(VmnetState *s) {
=C2=A0 =C2=A0 ssiz= e_t size;
=C2=A0 =C2=A0 while (s->packets_send_current_pos < s->= ;packets_send_end_pos) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 size =3D qemu_send_= packet_async(&s->nc,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 s->iov_buf[s->packets_send_current_pos].iov_base,=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->packets_b= uf[s->packets_send_current_pos].vm_pkt_size,
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vmnet_send_completed);
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 ++s->packets_send_current_pos;
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 if (size =3D=3D 0) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprin= tf(stderr, "stop\n"); // (!)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 s->send_enabled =3D false;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 return;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0= =C2=A0 fprintf(stderr, "ok\n");=C2=A0// (!)
=C2=A0 =C2=A0 }=C2=A0 =C2=A0 s->send_enabled =3D true;
}

static void vmnet_= send_completed(NetClientState *nc, ssize_t len) {
=C2=A0 =C2=A0 fprintf(= stderr, "continue\n"); // (!)
=C2=A0 =C2=A0 VmnetState = *s =3D DO_UPCAST(VmnetState, nc, nc);
=C2=A0 =C2=A0 vmnet_write_packets_= to_qemu(s);
=C2=A0 =C2=A0 if (s->send_enabled) {
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 qemu_bh_schedule(s->send_bh);
=C2=A0 =C2=A0 }
}

And the result (while both iperf3 + ping flood):
ok
ok<= br>ok
ok
ok
ok
ok
stop
continue
ok
ok
ok
stop=
continue
stop
continue
stop
continue
ok
ok
ok
s= top
continue

As you can see, if the packet = is sent immediately (ok),
the callback is not invoked. If 0 is returned = (stop),=C2=A0
the callback is invoked (continue) when QEMU is rea= dy.

Taking this into account, I don't see any way to = get rid
of the send_enabled flag without making implementationmore complex.

Best regards,
Vladislav Yaroshchuk

Regards,
Akihiko Odaki

> +
> +=C2=A0 =C2=A0 struct vmpktdesc packets_buf[VMNET_PACKETS_LIMIT];
> +=C2=A0 =C2=A0 int packets_send_current_pos;
> +=C2=A0 =C2=A0 int packets_send_end_pos;
> +
> +=C2=A0 =C2=A0 struct iovec iov_buf[VMNET_PACKETS_LIMIT];
>=C2=A0 =C2=A0} VmnetCommonState;
>=C2=A0 =C2=A0
> +const char *vmnet_status_map_str(vmnet_return_t status);
> +
> +int vmnet_if_create(NetClientState *nc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= xpc_object_t if_desc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= Error **errp);
> +
> +ssize_t vmnet_receive_common(NetClientState *nc,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0const uint8_t *buf,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size_t size);
> +
> +void vmnet_cleanup_common(NetClientState *nc);
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0#endif /* VMNET_INT_H */

--000000000000d5842f05da455b27--