Tested the current series of patches, mixed with patches from series
[1] and [2], and the virtio-net regression tests passed. I also tested
local VM migration under multiple NIC queues enabled and disabled, it
also passed.

[1] 
https://patchwork.ozlabs.org/project/qemu-devel/cover/[email protected]/
[2] 
https://patchwork.ozlabs.org/project/qemu-devel/cover/[email protected]/

Tested-by: Lei Yang <[email protected]>

On Wed, Sep 3, 2025 at 8:49 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> Hi all!
>
> Here is a refactoring of initialization code, to improve its
> readability and get rid of duplication.
>
> v3:
> - rebase on top of [PATCH 00/10] io: deal with blocking/non-blocking fds
> - improve some commit messages
> - add t-b marks by Lei Yang (hope, they still counts after rebasing :)
>
> v2:
> 01,03: improve commit msg
> 14: fix return value for new net_tap_init_one()
> 15: add return statements to other cases, to not break them
> 20: new
>
> Below are the initialization flow diagrams showing the changes.
>
> BEFORE REFACTORING:
> ==================
>
> ```
> net_init_tap()
>     |
>     +-- if (tap->fd)
>     |   +-- duplicated logic*
>     |   +-- net_init_tap_one()
>     |
>     +-- else if (tap->fds)
>     |   +-- for each fd:
>     |       +-- duplicated logic*
>     |       +-- net_init_tap_one()
>     |
>     +-- else if (tap->helper)
>     |   +-- duplicated logic*
>     |   +-- net_init_bridge()
>     |
>     +-- else (normal case)
>         +-- for each queue:
>             +-- net_tap_init()
>             +-- net_init_tap_one()
>
> net_init_bridge()
>     |
>     +-- duplicated logic*
>     +-- net_tap_fd_init()
>
> net_init_tap_one()
>     |
>     +-- net_tap_fd_init()
>
> net_tap_init()
>     |
>     +-- tap_open()
>
> net_tap_fd_init()
>     |
>     +-- qemu_new_net_client()
>     +-- Initialize TAPState
>
> * duplicated logic: set fd nonblocking + probe vnet_hdr
> ```
>
> AFTER REFACTORING:
> =================
>
> ```
> net_init_tap()
>     |
>     +-- if (tap->fd)
>     |   +-- net_tap_from_monitor_fd()
>     |
>     +-- else if (tap->fds)
>     |   +-- for each fd:
>     |       +-- net_tap_from_monitor_fd()
>     |
>     +-- else if (tap->helper)
>     |   +-- net_init_bridge()
>     |
>     +-- else (normal case)
>         +-- net_tap_open()
>
> net_tap_open()
>     |
>     +-- for each queue:
>         +-- net_tap_open_one()
>
> net_tap_open_one()
>     |
>     +-- tap_open()
>     +-- net_tap_fd_init_common()
>
> net_tap_from_monitor_fd()
>     |
>     +-- net_tap_fd_init_external()
>
> net_tap_fd_init_external()
>     |
>     +-- net_tap_fd_init_common()
>
> net_init_bridge()
>     |
>     +-- net_tap_fd_init_external()
>
> net_tap_fd_init_common()
>     |
>     +-- qemu_new_net_client()
>     +-- Initialize TAPState
> ```
>
> Solved problems:
>
> - duplicated logic to handle external
>   file descriptors (set nonblocking, probe vnet_hdr)
>
> - duplication between tap/helper case in
>   net_init_tap() and net_init_bridge()
>
> - confusing naming and functionality spread between functions (we had
>   net_init_tap(), together with net_tap_init(); also main central
>   function was net_init_tap_one(), and part of its logic (not clear
>   why) moved to separate net_tap_fd_init()),
>
> - net_init_tap() was just too big
>
> Vladimir Sementsov-Ogievskiy (19):
>   net/tap: net_init_tap_one(): add return value
>   net/tap: tap_set_sndbuf(): add return value
>   net/tap: net_init_tap_one(): drop extra error propagation
>   net/tap: net_init_tap_one(): move parameter checking earlier
>   net/tap: net_init_tap(): refactor parameter checking
>   net/tap: net_init_tap(): drop extra variable vhostfdname
>   net/tap: move local variables related to the latter case to else
>     branch
>   net/tap: use glib strings vector and g_strsplit for fds case
>   net/tap: drop extra tap_fd_get_ifname() call
>   net/tap: net_init_tap_one(): refactor to use netdev as first arg
>   net/tap: net_init_tap_one(): support bridge
>   net/tap: net_init_bridge(): support tap
>   net/tap: refactor net_tap_init() into net_tap_open_one()
>   net/tap: introduce net_tap_open()
>   net/tap: introduce net_tap_fd_init_external()
>   net/tap: introduce net_tap_from_monitor_fd() helper
>   net/tap: split net_tap_setup_vhost() separate function
>   net/tap: drop net_tap_fd_init()
>   net/tap: introduce net_init_tap_fds()
>
>  net/tap-linux.c |   5 +-
>  net/tap.c       | 553 ++++++++++++++++++++++--------------------------
>  net/tap_int.h   |   2 +-
>  3 files changed, 263 insertions(+), 297 deletions(-)
>
> --
> 2.48.1
>


Reply via email to