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 9C1BCA04AC; Fri, 1 May 2020 12:33:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3F0CC1DA06; Fri, 1 May 2020 12:33:28 +0200 (CEST) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id C1A6D1DA05 for ; Fri, 1 May 2020 12:33:26 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id h9so724087wrt.0 for ; Fri, 01 May 2020 03:33:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=Q9gVe+7KFboA57L73sRtPPuDkE0n2m0gd2oiKEQa/fc=; b=cagOl7485aiViL+opI+aEN1VVUO0kJEttWBihpzkd8M69vxdXYvi0Usg9H6Dlt7FIz ubOpjoqHZ22vpD9oRbCN8L7a6+CBlnyhbovFo3hdVQQ52xWY97WuUJ0LEzUxWezon+gB bIzo/ep9JZf8EDj7kVVVgj3SriWRHiEiH58ni8sZP29Nfh+RHW8xpSTIhokuNLt6ZrIf PSvxOuTTIFl1q+hPGPYt9dJL7pTmT4mzDe9wcoMgRf/c68y9B0H01eKDWE2wvGe6iOB7 NstuGWz5Bjql210xa8ypQGhOfx/HZ4eRTk0fvS2EN7Soe3vyjHD/KB2CSpPLfPg0wMoe GuAA== X-Gm-Message-State: AGi0PuYVGlaraDaYKQsUypPHB0+V4hO4Djoc76R/LBfOSRTNHzyFu6oT nUrG9QEXVyVx0JeDeyC3wKA= X-Google-Smtp-Source: APiQypK22okAu1sc44OZumtJQzJ+hlymubjPofowzPNroS08gimvGpaiBQgU3aBv4jPHLIOieiNFbg== X-Received: by 2002:adf:f342:: with SMTP id e2mr3353953wrp.146.1588329206363; Fri, 01 May 2020 03:33:26 -0700 (PDT) Received: from localhost ([2a01:4b00:f419:6f00:7a8e:ed70:5c52:ea3]) by smtp.gmail.com with ESMTPSA id h2sm4236425wro.9.2020.05.01.03.33.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 May 2020 03:33:25 -0700 (PDT) Message-ID: From: Luca Boccassi To: Dan Gora Cc: Mattias =?ISO-8859-1?Q?R=F6nnblom?= , "dev@dpdk.org" , David Marchand , Jerin Jacob Date: Fri, 01 May 2020 11:33:24 +0100 In-Reply-To: References: <20200421195446.1730-1-dg@adax.com> <20200421195446.1730-3-dg@adax.com> <5df15087-781e-d27f-b7d8-50b1b8cb0c94@ericsson.com> <80e5cf97-feae-c753-5c65-4f3b121729f3@ericsson.com> <9782db5b1e5d2dfdcfa8e52410fd166df5db938d.camel@debian.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.30.5-1.1 MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/2] eal: resolve getentropy at run time for random seed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, 2020-04-30 at 17:43 -0300, Dan Gora wrote: > On Thu, Apr 30, 2020 at 5:29 PM Luca Boccassi wrote: >=20 > > > > Adding a new dependecy happens only when building with the new vers= ion > > > > of the library. If it's not available, then there's no new dependen= cy. > > >=20 > > > But you also do not get to use the new getentropy() if you happen to > > > compile on a system which does not have the latest glibc, or if you > > > use the makefile system.. > >=20 > > And that's perfectly fine - backward compatibility workarounds are not > > a problem to me. > >=20 > > > > It sounds to me like you are trying to add workarounds for issues i= n > > > > your downstream build/deployment model, where your build dependenci= es > > > > are newer than your runtime dependencies. This in general is rarely > > > > well supported. > > >=20 > > > I am fully aware of that. I am not adding "workarounds", I am > > > eliminating unnecessary dependencies which probably never should have > > > been introduced in the first place. > >=20 > > It's not unnecessary. It's a better interface, and worth using if > > available. >=20 > "if available" is the key phrase here.. It not only has to be > available on the execution machine, it has to be available on the > compilation machine as well... Yes, same as every other dependency. > > > > Now I'm fine with adding workarounds as _fallbacks_ - what I am > > > > explicitly NACKing is forcibly restricting to the least common > > > > denominator because of issues in a third party build/deployment sys= tem > > > > that doesn't follow the common norm. > > >=20 > > > ugh.. this is the exact _opposite_ of what this patch does. It is no= t > > > restricting anything to a least common denominator. It is allowing > > > the DPDK to use the "best" available function, regardless of the buil= d > > > system. > > >=20 > > > Restricting to the least common denominator is what the original patc= h did... > >=20 > > This is restricting to the least common denominator of /dev/urandom, > > which is a bad interface, frail with issues that everybody is moving > > away from, in favour of the programmatic API that this patch is > > removing, in order to fix a corner case with a non-standard, third- > > party build system that downgrades dependencies at runtime vs build > > time. >=20 > Well, no, because rdseed is used first if available and /dev/urandom > is used next.. >=20 > And this is not a corner case at all.. There are lots of linux > distributions which DPDK claims to support which do not support > getentropy(). This is hardly a non-standard build system. You really > compile and support a separate binary for every possible system that > your customers will use? Yes, that's how Linux distributions work. At the very least, most deployments ensure dependencies are not downgraded, as that's rarely supported, for very good reasons. > And what is this about third parties? Last I checked DPDK was an API > framework, not a proprietary standalone application. It is designed, > by definition, to be used by "third parties". Or does it only have to > work in Intel's toolchains? Third party was the wrong expression. DPDK is a set of libraries with a build system (two actually) which links them against their dependencies. It is the reponsibility of who does deployment to make sure those dependencies, as established at build time, are satisfied at runtime. Up until now, downgrading at runtime vs build time was not supported as far as I know, so what you are effectively asking is to double the size of the support matrix, and for the project to ensure that every single dependency can always be built against a new version and used against an older one. > > > > This is especially true when dealing with RNG APIs, where the tinie= st > > > > and most innocent-looking mistake could have disastrous consequence= s. > > >=20 > > > This does not change the functionality of the RNG at all. It just > > > makes it work in the way that it was intended. These changes were > > > only introduced into 19.08, so they are not historical artifacts or > > > anything. > >=20 > > It's reimplementing a syscall using a different interface which has > > different semantics. A small mistake there is going to cost us dearly. >=20 > The code was copied almost verbatim from the getentropy() function in > glibc, it's hard to see it going that wrong there. The code can be > tested using the same test cases that the original patch used. I > don't see how there is a difference in test coverage here. And who's going to keep it updated as the implementation in glibc changes, if/when a bug is found there? Also, glibc is LGPL2.1 licensed, which means copying almost verbatim a non-trivial piece of code can possibly result in librte_eal as a whole to be covered under the terms of LGPL2.1 (IANAL disclaimer). This is unlikely to be well received, given it's BSD-3-clause now. > If it's such a big deal to use /dev/urandom, then what about my v3 > patch which used dlopen()/dlsym() to try to find getentropy()? Is > that not then acceptable? dlopen/dlsym() are used in several placed > in DPDK. I don't have a problem with that specifically, but it needs to be clear to the maintainers that what you are effectively asking is for all dependencies to support runtime-downgrading transparently and out of the box. This comes with a huge additional cost, and I do not believe it is supported at the moment. --=20 Kind regards, Luca Boccassi