From owner-svn-src-all@freebsd.org Fri May 29 18:33:21 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 74F3233E76D; Fri, 29 May 2020 18:33:21 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49YY9F2NVfz4C9V; Fri, 29 May 2020 18:33:21 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: from mail-qv1-f51.google.com (mail-qv1-f51.google.com [209.85.219.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) (Authenticated sender: kevans) by smtp.freebsd.org (Postfix) with ESMTPSA id 44D6B117DB; Fri, 29 May 2020 18:33:21 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: by mail-qv1-f51.google.com with SMTP id p15so1528289qvr.9; Fri, 29 May 2020 11:33:21 -0700 (PDT) X-Gm-Message-State: AOAM532Uys3IBFX+DrOat2Fm/XTy6R8m2jBs83prkjylqQ4jIgY7N9vC /gmoRms51pCFxOp8KqBEhs4od8Z1pyNIBqmWR0Y= X-Google-Smtp-Source: ABdhPJzBV2fxu2jLSX35r3Cwm2TvBARFE4jtGlaGmQfsfgsSisWs4rCg8Xu5lqCpqX+TJxJls0Y3c2RCF0nsyTYM1j4= X-Received: by 2002:a0c:ba22:: with SMTP id w34mr9488733qvf.129.1590777200782; Fri, 29 May 2020 11:33:20 -0700 (PDT) MIME-Version: 1.0 References: <202005201103.04KB3xTp013965@repo.freebsd.org> In-Reply-To: From: Kyle Evans Date: Fri, 29 May 2020 13:33:08 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r361275 - in head/sys: conf dev/hyperv/hvsock dev/hyperv/include dev/hyperv/vmbus modules/hyperv modules/hyperv/hvsock sys To: Wei Hu Cc: src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 May 2020 18:33:21 -0000 On Fri, May 29, 2020 at 12:08 PM Wei Hu wrote: > > -----Original Message----- > > > > [... snip ...] > > > > +void > > > > +hvs_trans_init(void) > > > > +{ > > > > + /* Skip initialization of globals for non-default instances. */ > > > > + if (!IS_DEFAULT_VNET(curvnet)) > > > > + return; > > > > + > > > > + if (vm_guest != VM_GUEST_HV) > > > > + return; > > > > + > > > > + HVSOCK_DBG(HVSOCK_DBG_VERBOSE, > > > > + "%s: HyperV Socket hvs_trans_init called\n", __func__); > > > > + > > > > + /* Initialize Globals */ > > > > + previous_auto_bound_port = MAX_PORT; > > > > + sx_init(&hvs_trans_socks_sx, "hvs_trans_sock_sx"); > > > > + mtx_init(&hvs_trans_socks_mtx, > > > > + "hvs_trans_socks_mtx", NULL, MTX_DEF); > > > > + LIST_INIT(&hvs_trans_bound_socks); > > > > + LIST_INIT(&hvs_trans_connected_socks); > > > > +} > > > > + > > > > > > I have a suspicion that all of these should really be per-vnet for > > > correct semantics with VIMAGE, with the IS_DEFAULT_VNET check earlier > > > dropped completely. I haven't read around the rest all that much, but > > > this would at least seem to prevent port re-use by a different vnet, > > > which is perhaps "good enough" but I think this is something that > > > should be fixed in the mid-term. > > > > > > > I have a follow-up concern about whether this is actually going to be > > maintained... it's been a full week with not even an acknowledgement or > > rebuttal of any of the concerns I've raised, with some of them being completely > > trivial to address in the short-term. I don't think that we really want this in the > > tree in its current state given this level of engagement. > > > Sorry for my late response, Kyle. I read your comments last week. To be honest I am > not familiar to VNET and VIMAGE, so I don't quite understand. I got distracted > into other work so my response to this was delayed. > > Do you mean to drop these two lines? > if (!IS_DEFAULT_VNET(curvnet)) > return > I copied these from other socket init code. If they are not necessary I can remove them. > Alright, let's rewind a little bit here. =-) Consider while reading the below that I have no idea what the host-side of these sockets look like beyond what I just very quickly skimmed from [0]. It's a little more involved than that; consider each vnet as its own independent network stack that's largely isolated from other vnets. The host starts out with a vnet, vnet0 (the default vnet), and root may spawn additional vnets attached to jails to allow the jail to operate its own network stack as well. Your pr_init will get called once per vnet spawned, including the obvious default vnet, which is why the check is there in the first place -- you may have some global state that only needs to be initialized once, at which point you would exclude non-default vnets from executing those bits. The more I think about it, though, the more I wonder if hvsock makes sense at all in non-default vnet contexts and whether hvs_trans_attach should really be attaching to sockets outside of the default vnet. You can bind in your FreeBSD HyperV guest to a port, and a host service's option for connecting is specifying guest GUID + port; there's not really any wiggle room for multiple entities within a guest to be binding to a given port anyways. It may very well be the case that the status quo is the optimal outcome. Given that, does it make sense that the host connects via the guest GUID and can potentially end up connected to some jail of unknown trust operating its own network stack? As a final scattered thought for the hour, does the Linux implementation of this stuff do anything to give a guest admin visibility into existing HyperV sockets on the system? AFAICT here, there's no sockstat integration or anything else that might export hvsock information to userland, so one's only option to tracking down whether a HyperV socket even exists and to which process it belongs would appear to be probing around in the kernel. Thanks, Kyle Evans [0] https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/user-guide/make-integration-service