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=fruqb3AQ; 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 E38B45A026F for ; Thu, 09 Jan 2025 16:36:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736437008; 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=G+8GfVaEb+wgPiw6X9YyIDSlbOR2SDUHE+siKKIYPQk=; b=fruqb3AQWqKlFOeBnTaeZnSKQTHBdRmCwJl/g4nh0dItRzfyREt/w4MUA0zRtgzGqThU9h J9C7kiQ6vErhb6/t9HtGTDl0HTZva4RKJ273pej2XVDPynl1LUZ6RYEFtYZgDPZduJNXs/ YA4Qk9qa7se3urgPBsSaMVyYpycj8+A= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-605-VauTNTaEPXKEEr7MFRlQGQ-1; Thu, 09 Jan 2025 10:36:46 -0500 X-MC-Unique: VauTNTaEPXKEEr7MFRlQGQ-1 X-Mimecast-MFC-AGG-ID: VauTNTaEPXKEEr7MFRlQGQ Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-3860bc1d4f1so730900f8f.2 for ; Thu, 09 Jan 2025 07:36:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736437005; x=1737041805; 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=G+8GfVaEb+wgPiw6X9YyIDSlbOR2SDUHE+siKKIYPQk=; b=b06RUVINGrWBQ3Xcwv67qTOdI1GkehebY4ex7sIG+vt90BYnehy51KDMXAGnmMM5ru FzexhYZg6ZDSQolfaLil09Gr7p/JhyXY7pe+OCQYgepuQzETNPIqZwpGdhceoj45Mxm/ qCXUqJvlrynQiX6pDy93CR+GpvYbDXkAOKmZUXJmUAX/R2zR3L9VCv11gdlttshzglQ6 AWj1Rz6zgmQw6BYks0omGqwFfOFUTsrW7FHvWuWqwwy129ao418FHXDSkkVgqRDs9tjj 3/FMbF1lRLP/CZFklnIGmZ2V7FTqdKx4mX6Xxyr19gbX9F2xorvOQRn9aiRhNtYG45iB rfCw== X-Gm-Message-State: AOJu0YyHmcnlY5SzCc48UYopEHfpPzZI2omf9z+nKmD2YxsNm7PixGsj 6sEmhtDuxySJHA3nxLigZNAegG1D9syKluQuAuXvh1IE2HOYIEoa0lCrGwp49g3kQe+pTX/nWp+ Pg/DQGV8MLl/rHublZlQ3EPqLLbpD+4GZ8YOTxdkKgkSy2x4PFQ== X-Gm-Gg: ASbGncuO/RDbiSs5A4AJEC+hoPK9Y7BoYjEk1QjYmmDHDBD9hyXg1k2jLlh07asGDBo 0Hd+Og/IXPqnj56WTxjdezzae2daSOPuKMli5SkuzcYqeT+tsymhEnLIgws76fbqRVlbcw0MGpq tmKjRWwdyB1iXZyyqeZlFP3C+PNMMjJrTtgp8qi1XnQtyCv2JjyW4oe21mhyKf08K+UAzFhAB9h x2ByuRDH7K9wKn+ne+m+uRkqRKfBH5UPAX9j5r2NVOc1xkz5l5emdGtBJUaXpgxpO10TgGX09t9 7YVzyygiHg== X-Received: by 2002:a05:6000:188c:b0:386:8ff:d20b with SMTP id ffacd0b85a97d-38a8730ce41mr6133197f8f.27.1736437005266; Thu, 09 Jan 2025 07:36:45 -0800 (PST) X-Google-Smtp-Source: AGHT+IHNoE1BHnTDhHfSgApbzo1xktI1kEimHWlyKEmNQLldgeVvsYaYa4gotABxcKElUQkSo7rnYA== X-Received: by 2002:a05:6000:188c:b0:386:8ff:d20b with SMTP id ffacd0b85a97d-38a8730ce41mr6133170f8f.27.1736437004861; Thu, 09 Jan 2025 07:36:44 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-436e2da7768sm60022765e9.5.2025.01.09.07.36.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jan 2025 07:36:44 -0800 (PST) Date: Thu, 9 Jan 2025 16:36:42 +0100 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH] checksum: fix checksum with odd base address Message-ID: <20250109163642.0a0bfcea@elisabeth> In-Reply-To: <20250109130648.326933-1-lvivier@redhat.com> References: <20250109130648.326933-1-lvivier@redhat.com> 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: C9-8o0rioEMAjUTFUmSjv8xorEpaSPNZCQNLce4_OHI_1736437005 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: SAFDIFD32HX2AOTPHEVDOBCEM6WOWOOT X-Message-ID-Hash: SAFDIFD32HX2AOTPHEVDOBCEM6WOWOOT 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, Mike Jones 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: [Cc'ed Mike who reported this] On Thu, 9 Jan 2025 14:06:48 +0100 Laurent Vivier wrote: > csum_unfolded() must call csum_avx2() with a 32byte aligned base address. > > To be able to do that if the buffer is not correctly aligned, > it splits the buffers in 2 parts, the second part is 32byte aligned and > can be used with csum_avx2(), the first part is the remaining part, that > is not 32byte aligned and we use sum_16b() to compute the checksum. > > A problem appears if the length of the first part is odd because > the checksum is using 16bit words to do the checksum. > > If the length is odd, when the second part is computed, all words are > shifted by 1 byte, meaning weight of upper and lower byte is swapped. > > For instance a 13 bytes buffer: > > bytes: > > aa AA bb BB cc CC dd DD ee EE ff FF gg > > 16bit words: > > AAaa BBbb CCcc DDdd EEee FFff 00gg > > If we don't split the sequence, the checksum is: > > AAaa + BBbb + CCcc + DDdd + EEee + FFff + 00gg > > If we split the sequence with an even length for the first part: > > (AAaa + BBbb) + (CCcc + DDdd + EEee + FFff + 00gg) > > But if the first part has an odd length: > > (AAaa + BBbb + 00cc) + (ddCC + eeDD + ffEE + ggFF) Thanks, this description is really helpful. > To avoid the problem, do not call csum_avx2() if the first part cannot > have an even length, and compute the checksum of all the buffer using > sum_16b(). > > This is slower but it can only happen if the buffer base address is odd, > and this can only happen if the binary is built using '-Os', and that > means we have chosen to prioritize size over speed. Reported-by: Mike Jones > Link: https://bugs.passt.top/show_bug.cgi?id=108 > Signed-off-by: Laurent Vivier > --- > checksum.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/checksum.c b/checksum.c > index 1c4354d35734..2fd6867cdf75 100644 > --- a/checksum.c > +++ b/checksum.c > @@ -452,7 +452,7 @@ uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init) > intptr_t align = ROUND_UP((intptr_t)buf, sizeof(__m256i)); > unsigned int pad = align - (intptr_t)buf; > > - if (len < pad) > + if (pad & 1 || len < pad) I'm fine applying this as it is, because the issue is quite nasty and we have this great commit message anyway, but for clarity, could we have a comment mentioning why we're doing this? Something like: /* Don't mix sum_16b() and csum_avx2() with odd padding lengths */ (I'm not quite satisfied with it but I find it better than nothing). > pad = len; > > if (pad) -- Stefano