From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f52.google.com (mail-pa0-f52.google.com [209.85.220.52]) by dpdk.org (Postfix) with ESMTP id 14BFC5A67 for ; Wed, 22 Apr 2015 21:01:30 +0200 (CEST) Received: by pacyx8 with SMTP id yx8so280623614pac.1 for ; Wed, 22 Apr 2015 12:01:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=bCiELfQUKvnCSm54U0t9HmSY3kNc1R+ILPVaBkF27v8=; b=aJVIij2gW/kxnjloqGqcDfaZZ6yiuHvZMmigN69q5bjNgBx0FC89ln3OsXD/phPUhG EzTDN9uZ9vcbw7e6ML/uglqS3y/c7eqj/91qxOOzEVKyK/UHOCRFI832V9HsFIqx03Bq JNIKgw8h4F0qO2mYdOXk2Dkl+l9DGTq+/s+JexAwI/xlvXIIDjSzYpXxAcmu13ZgKeR1 ajAS6j/ibwpW4Xb4xTDOYJPh3N0o7X/QqoIgLPHk8haTIXckZpQ3NOqslalQpSR+rR+/ FPIMim64SFD5J1VHib8F+MV8dUE0l9G3zkPNttQtREB2NEzpoxjPICNCAeSDIDQzwgXJ y7+g== X-Gm-Message-State: ALoCoQlURFVomNqNh+tLrceIaoCa6aKgP4KNdNtAlGVT7zkuAOfu+D0NFb9MREmyuWxtDalGfosl X-Received: by 10.66.129.174 with SMTP id nx14mr49093662pab.12.1429729289132; Wed, 22 Apr 2015 12:01:29 -0700 (PDT) Received: from urahara (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by mx.google.com with ESMTPSA id qe9sm5751818pbc.62.2015.04.22.12.01.28 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Apr 2015 12:01:28 -0700 (PDT) Date: Wed, 22 Apr 2015 12:01:33 -0700 From: Stephen Hemminger To: "Burakov, Anatoly" Message-ID: <20150422120133.44b27408@urahara> In-Reply-To: References: <1429554771-32365-1-git-send-email-stephen@networkplumber.org> <20150421103459.6b8094d0@urahara> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] vfio: don't silently drop VFIO support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Apr 2015 19:01:30 -0000 On Wed, 22 Apr 2015 08:59:51 +0000 "Burakov, Anatoly" wrote: > Hi Stephen, > > > > Hi Stephen, > > > > > > > The VFIO_PRESENT #define was a landmine and we hit it. > > > > The DPDK has a config system and it should be used rather than > > > > silently dropping a feature during build only to have it fail at run time. > > > > > > > > If VFIO is configured, and the kernel headers are not present the > > > > build should fail. Rather than leaving developers puzzling why the > > > > build system (with old kernel headers) produced non functioning DPDK > > > > and their system (with new kernel headers) produced correctly working > > DPDK. > > > > > > > > As a matter of policy, really no code should be looking at > > > > except for kernel drivers with compat files. > > > > > > In theory, I agree with you. In practice however, this change will > > unconditionally break builds on pre-VFIO kernels (<3.6). > > > > If someone is building with an older kernel, then they should change the > > config. > > > > Well, it's not like this is immediately obvious to anyone building DPDK. With your patch, the "out-of-the-box" experience is no longer there, since one now has to figure out why it's not building, go edit config files, et al, which is easy for you and me, but may not be easy for someone dealing with DPDK for the first time. > > I agree that both situations aren't ideal, it just seems to me that not building VFIO by default is better in that sense (EAL will show a warning message saying that VFIO isn't enabled, but the code will actually compile). Of course, the best of both worlds would be something like a configure script. > > Thanks, > Anatoly What we ended up doing is a second patch to have a "compat_vfio.h" file to allow building on older systems. I will send that as a follow on. The problem is compounded by the fact that VFIO or not has to be decided by startup scripts prior to starting the DPDK application. The startup scripts have no way of knowing that the DPDK application was built with broken VFIO support. That is why I think the original method with VFIO_PRESENT was not the correct way to handle this.