From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C8EFFA04BC for ; Thu, 8 Oct 2020 11:21:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 845DE1BBD1; Thu, 8 Oct 2020 11:21:36 +0200 (CEST) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 48AE41BBD1 for ; Thu, 8 Oct 2020 11:21:34 +0200 (CEST) Received: by mail-wm1-f65.google.com with SMTP id d3so5748586wma.4 for ; Thu, 08 Oct 2020 02:21:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=+2kUBDpl7YdeBbSYWqCBYynYRiXhq1Q7/YXKAvdPIcE=; b=dNl6BSEpvQ4pSraGZf4oIjKu8UYroMyEzzTrHY6PhGWYgrvQzkNvkbkHxC9m2LiVUx SUX6GliSoP/aruHy0wN0a1s8VLkrOp3AMKxYQXfEHW478zco8LDIazoGIAfwNxD+zxsB GpEutkpmDNXSGAWkcsrn+CsbAADOPb/i0dL9y2Rt9GkQyQwRcD76fybkPTRsmcf1loFs 0q9/oXS9lWUaVrclgvmxDKmDL6pjEcYJwYEOozaK30d4RvEYsWCwTEysrwFSajmFBtFE X36xv7oUC/lIc81umtZuigxAbMs6vU6dca4eZCcYg0nudSlch5UWcYEQBiTDUkgPPJOw 2wmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=+2kUBDpl7YdeBbSYWqCBYynYRiXhq1Q7/YXKAvdPIcE=; b=Rp/cJmMerdOQsGZpQeMYatdm1EASjMHY22tSNbZWbZnSxAU3DYWdp6ADMO7Y50eLro CS03fQcjyp2o6X/tlFVIJ5zsgJsffLz6kCboaDg3p92xVodGRG0sLLEucF/zI/OaJ/S6 vXroBoJow37W9iEMejPhAJ/Xu4w7Q3DP9kqd80crpwsH5uL2Y3nLm1Crwmum+n/Lr27s Rp9mUGelguhTZTsHYBG8TH0THNYruv+SGrdP9hkfNc3DcJdAlbZ7o1bqsL1IrzvLjahg QG9YxuWM596rf0X44EuwNiwClpeOmlCMcpVjPNgewiLfCE/z1rEJB+N1DxbBIHUBptli aXNQ== X-Gm-Message-State: AOAM530pc0V3DNvwMCKPKkfbQsRSfTB6pbEDnlbiTngtlQyy7ewzLN8L FjT90llq0CIXEb5vpOuFDRUwmA== X-Google-Smtp-Source: ABdhPJx5FLXBg79rorxELVR6YmLRra/53WUkrCaZYV60nB3uGujaZJu0B2fGZY0WQJkS3JTFs5guKw== X-Received: by 2002:a1c:4e1a:: with SMTP id g26mr7679922wmh.98.1602148892871; Thu, 08 Oct 2020 02:21:32 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id i3sm6355251wrs.4.2020.10.08.02.21.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2020 02:21:31 -0700 (PDT) Date: Thu, 8 Oct 2020 11:21:31 +0200 From: Olivier Matz To: "Zhang, Roy Fan" Cc: "dev@dpdk.org" , "Kovacevic, Marko" , Akhil Goyal , "Kusztal, ArkadiuszX" , "stable@dpdk.org" , Anoob Joseph Message-ID: <20201008092131.GU21395@platinum> References: <20201006074143.31691-1-olivier.matz@6wind.com> <20201006074143.31691-3-olivier.matz@6wind.com> <20201006100901.GJ21395@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-stable] [PATCH 2/3] examples/fips_validation: ignore \r in input files X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, On Thu, Oct 08, 2020 at 08:50:25AM +0000, Zhang, Roy Fan wrote: > Hi Olivier, > > Anood and us had the similar discussion. > > Can we change the sample application to parse version data instead, > and for the version specific code changes we will wrap them by a > branch to compare the parsed version and the expected version? > (we probably should have done that long time ago). > > I drafted a code change to parse the version data, see if you think it > is ok? Thank you for your feedback. The code that gets the version looks good to me (I just have a small comment, see below). However I'm not sure what to do with it. Do you mean we should return an error if the version is incorrect? Or should we only skip '\r' for old versions? FIPS_DEF_VERSION is not used in your patch. In that case, I think it is a bit overkill. Do you think it is a problem to always drop '\r'? If you think we should not support files containing '\r', I'm fine with it, I can drop this particular patch. > > diff --git a/examples/fips_validation/fips_validation.c b/examples/fips_validation/fips_validation.c > index 9bdf257b8..9b6518c92 100644 > --- a/examples/fips_validation/fips_validation.c > +++ b/examples/fips_validation/fips_validation.c > @@ -98,7 +98,7 @@ fips_test_parse_header(void) > uint32_t i; > char *tmp; > int ret; > - int algo_parsed = 0; > + int algo_parsed = 0, version_parsed = 0; > time_t t = time(NULL); > struct tm *tm_now = localtime(&t); > > @@ -107,6 +107,27 @@ fips_test_parse_header(void) > return ret; > > for (i = 0; i < info.nb_vec_lines; i++) { > + /* parse the version info */ > + tmp = strstr(info.vec[i], "CAVS "); > + if (tmp != NULL) { > + if (version_parsed != 0) { > + RTE_LOG(ERR, USER1, > + "Multiple version data\n"); > + return -1; > + } > + > + tmp = tmp + sizeof("CAVS "); I think it should be strlen(), because sizeof() will contain the '\0'. Or it could be sizeof() - 1. > + > + if (strlen(tmp) >= MAX_VER_STRING_SIZE) { > + RTE_LOG(ERR, USER1, "Version (%s) too long\n", > + tmp); > + return -1; > + } > + > + strlcpy(info.version, tmp, MAX_VER_STRING_SIZE); > + version_parsed = 1; > + } > + > if (!algo_parsed) { > if (strstr(info.vec[i], "AESVS")) { > algo_parsed = 1; > diff --git a/examples/fips_validation/fips_validation.h b/examples/fips_validation/fips_validation.h > index 75fa555fa..b8c60c55f 100644 > --- a/examples/fips_validation/fips_validation.h > +++ b/examples/fips_validation/fips_validation.h > @@ -15,6 +15,9 @@ > #define MAX_BUF_SIZE 2048 > #define MAX_STRING_SIZE 64 > #define MAX_DIGEST_SIZE 64 > +#define MAX_VER_STRING_SIZE 8 > + > +#define FIPS_DEF_VERSION "21.0" > > #define POSITIVE_TEST 0 > #define NEGATIVE_TEST -1 > @@ -155,6 +158,7 @@ struct sha_interim_data { > }; > > struct fips_test_interim_info { > + char version[MAX_VER_STRING_SIZE]; > FILE *fp_rd; > FILE *fp_wr; > enum file_types file_type; > > > Regards, > Fan > > > -----Original Message----- > > From: Olivier Matz > > Sent: Tuesday, October 6, 2020 11:09 AM > > To: Zhang, Roy Fan > > Cc: dev@dpdk.org; Kovacevic, Marko ; Akhil > > Goyal ; Kusztal, ArkadiuszX > > ; stable@dpdk.org; Anoob Joseph > > > > Subject: Re: [PATCH 2/3] examples/fips_validation: ignore \r in input files > > > > Hi Fan, > > > > On Tue, Oct 06, 2020 at 08:47:10AM +0000, Zhang, Roy Fan wrote: > > > Hi Olivier, > > > > > > > -----Original Message----- > > > > From: Olivier Matz > > > > Sent: Tuesday, October 6, 2020 8:42 AM > > > > To: dev@dpdk.org > > > > Cc: Kovacevic, Marko ; Akhil Goyal > > > > ; Zhang, Roy Fan ; > > Kusztal, > > > > ArkadiuszX ; stable@dpdk.org > > > > Subject: [PATCH 2/3] examples/fips_validation: ignore \r in input files > > > > > > > > Some test vectors contain '\r' before '\n' (see link). Ignore them. > > > > > > > > Link: https://www.openssl.org/docs/fips/testvectors-linux-2007-10- > > 10.tar.gz > > > > Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS > > application") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Olivier Matz > > > > --- > > > > examples/fips_validation/fips_validation.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/examples/fips_validation/fips_validation.c > > > > b/examples/fips_validation/fips_validation.c > > > > index 13f763c9aa..858f581ba3 100644 > > > > --- a/examples/fips_validation/fips_validation.c > > > > +++ b/examples/fips_validation/fips_validation.c > > > > @@ -33,6 +33,8 @@ get_file_line(void) > > > > > > > > if (loc >= MAX_LINE_CHAR - 1) > > > > return -ENOMEM; > > > > + if (c == '\r') > > > > + continue; > > > > if (c == '\n') > > > > break; > > > > line[loc++] = c; > > > > -- > > > > > > > > > The patch looks ok but the test file link you provided in the patch is CAVS > > > 5.3. > > > > > > As mentioned in > > > https://doc.dpdk.org/guides/sample_app_ug/fips_validation.html, the > > supported > > > CAVS supported version is 21.0 (not latest one by newer than 5.3). In CAVS > > > 21.0 test files there is no '\r' before '\n' (I suppose this is for Windows > > > right). > > > > Thank you for your feedback. > > > > I'm ok to drop this patch from the patchset if you feel it's useless, or > > I can update the commit log with the information you provide, to clarify > > that it should not happen with the supported version of CAVS. > > > > Please let me know what you prefer. > > > > > > Thanks, > > Olivier