From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] net/nfp: add CPP bridge as service Date: Fri, 11 Jan 2019 11:48:10 +0000 Message-ID: <9ffc5c5b-ce11-331d-b82c-ccbdab4d5350@intel.com> References: <20190103085621.23611-1-alejandro.lucero@netronome.com> <09fd5132-905b-bbe0-1178-392595027353@intel.com> <9afed691-6710-7efd-09e7-ae927cc091a8@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: dev To: Alejandro Lucero Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 472711B99F for ; Fri, 11 Jan 2019 12:48:13 +0100 (CET) In-Reply-To: Content-Language: en-US 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 1/10/2019 11:55 AM, Alejandro Lucero wrote: > > > On Wed, Jan 9, 2019 at 4:15 PM Ferruh Yigit > wrote: > > On 1/9/2019 2:20 PM, Alejandro Lucero wrote: > > > > > > On Wed, Jan 9, 2019 at 10:54 AM Ferruh Yigit > > >> wrote: > > > >     On 1/3/2019 8:56 AM, Alejandro Lucero wrote: > >     > The Netronome's Network Flow Processor chip is highly programmable > >     > with the goal of processing packets at high speed. Processing units > >     > and other chip components are available from the host through the > >     > PCIe CPP(Command Push Pull bus) interface. The NFP PF PMD configures > >     > a CPP handler for setting up and working with vNICs, perform actions > >     > like link up or down, or accessing extended stats from the MAC > component. > >     > > >     > There exist NFP host tools which access the NFP components for > >     > programming and debugging but they require the CPP interface. When the > >     > PMD is bound to the PF, the DPDK app owns the CPP interface, so these > >     > host tools can not access the NFP through other means like NFP kernel > >     > drivers. > >     > > >     > This patch adds a CPP bridge using the rte_service API which can be > >     > enabled by a DPDK app. Interestingly, DPDK clients like OVS will not > >     > enable specific service cores, but this can be performed with a > >     > secondary process specifically enabling this CPP bridge service and > >     > therefore giving access to the NFP to those host tools. > > > >     Hi Alejandro, > > > > > > Hi Ferruh, > >   > > > >     Getting a few build errors, more details below. > > > >     > > >     > Signed-off-by: Alejandro Lucero > >      >> > >     <...> > > > >     > +     /* Obtain target's CPP ID and offset in target */ > >     > +     cpp_id = (offset >> 40) << 8; > > > >     With icc, i686 getting [1], it seems 'off_t' is 32bits long on 32bit > build. > > > >     [1] > >     error #63: shift count is too large > > > > > > We do not support 32 bits. I thought our PMD was not built in that case. > > If PMD doesn't support 32 bits, above is OK, I will update my script > accordingly. > > >   > > > >     <...> > > > >     > +                     if (err != (int)len) { > >     > +                             printf("%s: error when receiving, %d > of %lu\n", > >     > +                                     __func__, err, count); > > > >     Giving build error for 32bits [3], and can you please use logging > macros instead > >     of printf? > > > > > > Sure. > >   > > > >     [3] > >     error: format ‘%lu’ expects argument of type ‘long unsigned int’, but > argument 4 > >     has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]  > > > >     <...> > > > >     > +     /* Obtain target's CPP ID and offset in target */ > >     > +     cpp_id = (offset >> 40) << 8; > > > >     Same as above [1]. > > > >     <...> > > > >     > +                     if (err != (int)len) { > >     > +                             printf("%s: error when sending: %d of > %lu\n", > >     > +                                     __func__, err, count); > > > >     Same build error with above [3]. > > > >     <...> > > > >     > +nfp_cpp_bridge_serve_ioctl(int sockfd, struct nfp_cpp *cpp) > >     > +{ > >     > +     int cmd, err; > >     > +     uint32_t ident_size, tmp; > >     > + > >     > +     /* Reading now the IOCTL command */ > >     > +     err = recv(sockfd, &cmd, 4, 0); > >     > +     if (err != 4) { > >     > +             printf("%s: read error from socket\n", __func__); > >     > +             return -EIO; > >     > +     } > >     > + > >     > +     /* Only supporting NFP_IOCTL_CPP_IDENTIFICATION */ > >     > +     if (cmd != NFP_IOCTL_CPP_IDENTIFICATION) { > > > >     Giving build error with ppc_64-power8-linuxapp-gcc [2]. > > > > > > We do not support power architecture. > > Yes but issue seems not exactly ppc issue, more like signed - unsigned > comparison. Can you please check if is there any valid issue here? > > > This is a funny one. NFP_IOCTL_CPP_IDENTIFICATION is not zero, and cmd could be > anything.  > And it does work with other compilers! > > Talking with a compiler guy in the office, and it is hard to know why the > compiler is triggering an error here. I suspect this is some sort of endianness > mess, and he thinks the compiler could be assuming the cmd variable after recv > call is always negative or positive, and the macro always being the opposite in > powerpc, so the comparison is always true, what is what the error message says. > > Anyway, it is not clear how to fix this. Maybe defining cmd as uint32_t could > help. Any change we can test this before sending another patch version? I am using a cross compiler for ppc, it is freely available, you should be able to get and test with it, or I can test for you if you prefer. > >   > > >   > > > >     [2] > >     error: comparison is always true due to limited range of data type > >     [-Werror=type-limits] > > >