From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Z0vkaYd1; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 7FAD55A0272 for ; Fri, 20 Dec 2024 10:51:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734688299; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zLZOEd2mZR0K6rZLxckoKJx0bzxgSBtvFkKAJLTFkPI=; b=Z0vkaYd1oW5eGNoWmvUtLAbr4e3kplDEPoGTUIuvPchMHxKycE5VLthBxe2GfIrB84XHxx cIs7vMpyBpxsCeotmswtJLEpigtx7RGnUfBlVACktpkq/Yx/rrngKYip0vZis6gsyTjfOt TvsPgw3R9E+dJKdKxFpK9RtevEdjQWI= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-589-g_MY03iuM3-k4n-lLH7sVQ-1; Fri, 20 Dec 2024 04:51:38 -0500 X-MC-Unique: g_MY03iuM3-k4n-lLH7sVQ-1 X-Mimecast-MFC-AGG-ID: g_MY03iuM3-k4n-lLH7sVQ Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-43651b1ba8aso13441965e9.1 for ; Fri, 20 Dec 2024 01:51:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734688296; x=1735293096; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=zLZOEd2mZR0K6rZLxckoKJx0bzxgSBtvFkKAJLTFkPI=; b=PW/Sc5JSp1yHSBisRbVOZUqtRlCrYniwQ1D8CUzXUKSCg1c8WZzTWItXjOborjJ957 VNmjQ6QW26RtuxPB2IM356ALdPeB6/Jx12Xf7MdjlaHrlGqnYzqLLLT3/j6wXZhabuEB QuLTsoFTXRKUzBZUuideeE3iFPgyOxTchKtDtsWJD5BIbxSEgs9MsJ5+xc/2qq7xlGBc KpzgY/8IfK48+5ClwAkfgJ0SeZ5D8uvveb/haNyzlAfOxIGFpIVwLVoaFNljneSPrtNp 6h1/AVykkuoJ4tfLYXqrqc52iyekXSdCKss1PWQhNvsrXTdr/4o4N+okYlo8lxnieSIF w6OA== X-Gm-Message-State: AOJu0YxUrGRQbADimccRgPkUWuUuDXaoMM9h2I2eScTzrhbkLkJ7RCKD lZQTaJFUhUnraK+ShNWyJ6X+tiAVA5/rPfcMMFKkGy94lzINJSgx9VvaW7o5M/vWfXOiJXabXrg 23F9/t2qZ19bBFJ9YpYWvBzfLZYloKZnIlOPDzq3kktdy0BesOlnWZjP92Q== X-Gm-Gg: ASbGncuGUaSSsFqBNGY/lc3wXoEqOlPt6385R3oLfv+KOWG66ukaDBwpruuipJIUooT XQ57xf4UPwuYfJZLTGaJ8pZi4h+w2DK9x3U3X5BXRLRLSoGIT83fkjHLltOxqymm67NC7XcXS5H sWXKKK0PHkLdj/ukh6Wbn7Ma+V141yNJjkqBJ5N22WmBhyk+NpIYa/q+/QV8B4QrQ4DTYM04mUz hm8ToyAhg+x8V/kB/tPE0GdefP2bsudpq2pnyqpNm9gb1Rh5fZjgegVF0sctU/vH4RBlD9CtYmB z5hWoBLz9A== X-Received: by 2002:a5d:648d:0:b0:387:8604:5023 with SMTP id ffacd0b85a97d-38a223f5d5fmr2404374f8f.44.1734688296283; Fri, 20 Dec 2024 01:51:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IHO2WUOklNtAzRQEuyXDJO3ZsYJxChIK7oiT+3W3x4nY4wyzdhY5ypnNGbWYp9ZWbH+5/IprA== X-Received: by 2002:a5d:648d:0:b0:387:8604:5023 with SMTP id ffacd0b85a97d-38a223f5d5fmr2404333f8f.44.1734688295867; Fri, 20 Dec 2024 01:51:35 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c828f8fsm3592526f8f.12.2024.12.20.01.51.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Dec 2024 01:51:34 -0800 (PST) Date: Fri, 20 Dec 2024 10:51:33 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 2/3] packet: Don't have struct pool specify its buffer Message-ID: <20241220105133.6f6ee3d6@elisabeth> In-Reply-To: References: <20241213120156.4123972-1-david@gibson.dropbear.id.au> <20241213120156.4123972-3-david@gibson.dropbear.id.au> <20241219100011.5faed9fd@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: _3T_X5q4m9Fo10QIsZhAOmxijGk7gxIClUcs4SlsLn0_1734688297 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: ICPO26CQX2XOZHA2UN2RVMEPX2EYOITE X-Message-ID-Hash: ICPO26CQX2XOZHA2UN2RVMEPX2EYOITE X-MailFrom: sbrivio@redhat.com 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: On Fri, 20 Dec 2024 11:59:09 +1100 David Gibson wrote: > 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: > > > > > 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 the > > > vu_dev_region array in which the packets sit. Originally that made sense > > > because we stored each packet as an offset and length within that buffer. > > > > > > However dd143e389 ("packet: replace struct desc by struct iovec") replaced > > > 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. > > > > > > Similarly the old representation used a 16-bit length, so there were some > > > checks that packets didn't exceed that. That's also no longer necessary > > > with the struct iovec which uses a size_t length. > > > > > > I think under an unlikely set of circumstances it might have been possible > > > to hit that 16-bit limit for a legitimate packet: other parts of the code > > > place a limit of 65535 bytes on the L2 frame, however that doesn't include > > > the length tag used by the qemu socket protocol. That tag *is* included in > > > the packet as stored in the pool, however, meaning we could get a 65539 > > > byte packet at this level. > > > > 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. > > > > 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. > > > > 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. Oh, sorry, I was deep in the perspective of trying to make things crash... and it didn't do anything, just accepted the setting and kept sending packets out. Let me try that then, with and without your new series... -- Stefano