On 03.09.25 08:19, Jason Wang wrote:
On Sun, Aug 24, 2025 at 12:03 AM Vladimir Sementsov-Ogievskiy <[email protected]> wrote:This is needed for further unification of bridge initialization in net_init_tap() and net_init_bridge(). Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> --- net/tap.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/net/tap.c b/net/tap.c index eab0a86173..468dae7004 100644 --- a/net/tap.c +++ b/net/tap.c @@ -692,15 +692,18 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr, #define MAX_TAP_QUEUES 1024 -static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer, +static int net_init_tap_one(const Netdev *netdev, NetClientState *peer, const char *model, const char *name, const char *ifname, const char *script, const char *downscript, const char *vhostfdname, int vnet_hdr, int fd, Error **errp) { + const NetdevTapOptions *tap = &netdev->u.tap; TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr); int vhostfd; + assert(netdev->type == NET_CLIENT_DRIVER_TAP); +I think we should try our best to avoid assertions and return errors here.
Hmm. But why? This is a static function that we call only for TAP. The error here is not logically possible. The error here would mean: - either we have a terrible bug in code, so we don't understand which functions call which ones, so better to stop here (abort) - or it's a kind of memory corruption, and again, better to abort than continue damage users data The assert may be removed not breaking the logic. But it brings the best kind of documentation, that in this function we work only with TAP. Finally, there a lot of similar asserts already in net code and in tap.c: git grep 'assert.*== NET_CLIENT' | wc -l 45
if (tap_set_sndbuf(s->fd, tap, errp) < 0) { goto failed; } @@ -826,7 +829,7 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } - ret = net_init_tap_one(tap, peer, "tap", name, NULL, + ret = net_init_tap_one(netdev, peer, "tap", name, NULL, NULL, NULL, tap->vhostfd, vnet_hdr, fd, errp); if (ret < 0) { @@ -875,7 +878,7 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } - ret = net_init_tap_one(tap, peer, "tap", name, NULL, + ret = net_init_tap_one(netdev, peer, "tap", name, NULL, NULL, NULL, vhost_fds ? vhost_fds[i] : NULL, vnet_hdr, fd, errp); @@ -905,7 +908,7 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } - ret = net_init_tap_one(tap, peer, "bridge", name, NULL, + ret = net_init_tap_one(netdev, peer, "bridge", name, NULL, NULL, NULL, tap->vhostfd, vnet_hdr, fd, errp); if (ret < 0) { @@ -946,7 +949,7 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } - ret = net_init_tap_one(tap, peer, "tap", name, ifname, + ret = net_init_tap_one(netdev, peer, "tap", name, ifname, i >= 1 ? "no" : script, i >= 1 ? "no" : downscript, tap->vhostfd, vnet_hdr, fd, errp); -- 2.48.1
-- Best regards, Vladimir
