From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202412 header.b=Rq8P1y50; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 07D955A061D for ; Fri, 20 Dec 2024 02:13:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1734657205; bh=qBBXoMNV5IZVQMfsePP13fKg0p+IYJ2F2dLF+rP1gV0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Rq8P1y50Po0m1Vv40lSriV6zSEvlU4zO10jW44i8TvDH+hD1MoapJQenfjme7/Td7 Iv20SSX9XVgNCDsQqYnsdLfq6rDcL9kr7P+VWgxqI/dBQQZ3K2Rj1jpq/bUpYAJp5s aZWnYOMYmo6MXaZCoYbbKN0HfVpttDLD/9MPCrOnyYQfw+MzbCRmru8FNbx4cHy36Y +RqKn6XYRbvApP6HuESOZIldry03tjmZnd15dhR7VogduOIhi2QFqPFVMc8UoNcHf7 LbJItVQXLhtSAjU27TyPav6YtLka9l35FvWmwUzFVYAwH9Glq21A13h0n7vC5a8TTY EYh6+Kogynkrw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4YDqDP4Dndz4x3q; Fri, 20 Dec 2024 12:13:25 +1100 (AEDT) Date: Fri, 20 Dec 2024 11:59:09 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 2/3] packet: Don't have struct pool specify its buffer Message-ID: References: <20241213120156.4123972-1-david@gibson.dropbear.id.au> <20241213120156.4123972-3-david@gibson.dropbear.id.au> <20241219100011.5faed9fd@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hl/sOwgK4r3cWUr2" Content-Disposition: inline In-Reply-To: <20241219100011.5faed9fd@elisabeth> Message-ID-Hash: 2X2Y6UBXYOGZIKGKTXALQCTLG5R3K63X X-Message-ID-Hash: 2X2Y6UBXYOGZIKGKTXALQCTLG5R3K63X X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --hl/sOwgK4r3cWUr2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 19, 2024 at 10:00:11AM +0100, Stefano Brivio wrote: > On Fri, 13 Dec 2024 23:01:55 +1100 > David Gibson wrote: >=20 > > struct pool, which represents a batch of packets includes values giving > > the buffer in which all the packets lie - or for vhost_user a link to t= he > > vu_dev_region array in which the packets sit. Originally that made sen= se > > because we stored each packet as an offset and length within that buffe= r. > >=20 > > However dd143e389 ("packet: replace struct desc by struct iovec") repla= ced > > the offset and length with a struct iovec which can directly reference a > > packet anywhere in memory. This means we no longer need the buffer > > reference to interpret packets from the pool. So there's really no need > > to check where the packet sits. We can remove the buf reference and all > > checks associated with it. As a bonus this removes the special case for > > vhost-user. > >=20 > > Similarly the old representation used a 16-bit length, so there were so= me > > checks that packets didn't exceed that. That's also no longer necessary > > with the struct iovec which uses a size_t length. > >=20 > > I think under an unlikely set of circumstances it might have been possi= ble > > to hit that 16-bit limit for a legitimate packet: other parts of the co= de > > place a limit of 65535 bytes on the L2 frame, however that doesn't incl= ude > > the length tag used by the qemu socket protocol. That tag *is* include= d in > > the packet as stored in the pool, however, meaning we could get a 65539 > > byte packet at this level. >=20 > As I mentioned in the call on Monday: sure, we need to fix this, but at > the same time I'm not quite convinced that it's a good idea to drop all > these sanity checks. >=20 > Even if they're not based on offsets anymore, I think it's still > valuable to ensure that the packets are not exactly _anywhere_ in > memory, but only where we expect them to be. >=20 > If it's doable, I would rather keep these checks, and change the ones > on the length to allow a maximum value of 65539 bytes. I mean, there's > a big difference between 65539 and, say, 4294967296. Right, I have draft patches that do basically this. > By the way, I haven't checked what happens with MTUs slightly bigger > than 65520 bytes: virtio-net (at least with QEMU) doesn't budge if I > set more than 65520, but I didn't actually send big packets. I'll try > to have a look (also with muvm) unless you already checked. I'm not sure what you mean by "doesn't budge". No, I haven't checked with either qemu or muvm. There could of course be limits applied by either VMM, or by the guest virtio-net driver. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --hl/sOwgK4r3cWUr2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmdkwVwACgkQzQJF27ox 2GdkHg/+J0EC0U+2uUN0mlfNHeQf3JQSZohUgq/zxseQDy6xSqDDmBGeI/HwTc91 VQv9tK3jg+EiJu0VbwZ4TBbNhaS7MyR1CLI9jorbWIxnZGUJy9AhUNLadCGWicvb 38gt/I3QlthXgVvF6Uj4HOCn5qzeRkcGh6H7jsfccRJ9SIgrCJieYdMtR56e8Z4l g3kTuLc5dNfRwkpF/YfqniWIXTstFgjMqkDZE5NOLR+h2ngaiBgdepcPqYqZgWpa N4gq5rUMOdBjLeAnH/LdWwrv1TIxMAahcOjPYZG0nNq34I4xO+HwLebiASSiFscj TWYWRt6KZ8eyqZTNpC424pQcO5XuYLNnyjypiYPfX2pFdHceEZkm0xl68oUOmBeO 3JAzhq3Ziee1rb/DoFKnK4GW1rObMbZEBF8dOKL1hBBo1LvUXYgJB0zU3f5v2qnu ViycGQSUVd11UL56DU0AQEjVS6spmiLtok54dKDr6qyFaxMpG5FAD0uHxK4OEhNC tkIH3i9JAjh5UTyXiSiX/CQDaR6nmH4mprn5kdoy1yNg5jsSoH226JNQArmxwBgQ 6giziKS5sUQTIViGL9HNpcPHI6Tbcp+PsQiOCk2BRy34bA0NKfxGtw3/GyQ5yB3G 7JwV1nTEPLixaIcNvqlAAGYvftfxekBvmGFD1Rv9TYoYQPgHuaY= =SRlH -----END PGP SIGNATURE----- --hl/sOwgK4r3cWUr2--