Ping!)
Understand, that it's quite big. I can split into 2-3 series, if this helps.
On 23.08.25 19:03, Vladimir Sementsov-Ogievskiy wrote:
Hi all!
Here is a refactoring of initialization code, to improve its
readability and get rid of duplication.
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 (20):
net/tap: net_init_tap_one(): add return value
net/tap: add set_fd_nonblocking() helper
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 | 578 +++++++++++++++++++++++-------------------------
net/tap_int.h | 2 +-
3 files changed, 277 insertions(+), 308 deletions(-)
--
Best regards,
Vladimir
