Re: [PATCH] net/nfp: add CPP bridge as service

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/14/2019 6:00 PM, Alejandro Lucero wrote:
> 
> 
> On Mon, Jan 14, 2019 at 10:40 AM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 1/13/2019 9:41 PM, Thomas Monjalon wrote:
>     > 11/01/2019 17:42, Ferruh Yigit:
>     >> On 1/11/2019 1:25 PM, 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.
>     >>>
>     >>> v2:
>     >>>  - Avoid printfs for debugging
>     >>>  - fix compilation problems for powerpc
>     >>>
>     >>> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com
>     <mailto:alejandro.lucero@netronome.com>>
>     >>
>     >> Applied to dpdk-next-net/master, thanks.
>     >
>     > It does not compile with 32-bit toolchain.
>     >
>     > Please check the occurences of %lu, thanks.
> 
>     Hi Thomas,
> 
>     We aware the build error, but let it because nfp doesn't support 32-bit.
> 
>     But I just recognized that it is enabled by default on 32-bit default configs,
>     we should disable them.
> 
> 
>     Hi Alejandro,
> 
>     Can you please disable nfp driver explicitly on
>     'defconfig_i686-native-linuxapp-*' config files, perhaps also on
>     'defconfig_x86_x32-native-linuxapp-gcc' too?
> 
>     I will drop the existing patch from next-net.
> 
> 
> Ok. I'll do asap.
>  
> 
>     And if it is possible to fix the build error, specially if it is just for %lu of
>     the logging, I prefer the fix against the config update, but it is up to you.
> 
> 
> I did not see any logging error/warning when compiling nor any when using
> checkpatch. I have used a gcc 7.3.1 (Ubuntu) and a 8.2.1 (RH). What are you
> using for triggering such error?

Using 'i686-native-linuxapp-gcc' config, which is for 32-bit, gives following
build error [1] with this patch.

This is mainly just because logging and can be fixed by using %z for size_t and
use PRIx64 similar macros for fixed size variables.

For "cpp_id = (offset >> 40) << 8;" build error you need to use fixed size
version of the variable, like "uint64_t offset"

[1]
.../drivers/net/nfp/nfp_net.c: In function ‘nfp_cpp_bridge_serve_write’:
.../drivers/net/nfp/nfp_net.c:2979:32: error: format ‘%lu’ expects argument of
type ‘long unsigned int’, but argument 6 has type ‘unsigned int’ [-Werror=format=]
   sizeof(off_t), sizeof(size_t));
                                ^
.../build/include/rte_log.h:324:25: note: in definition of macro ‘RTE_LOG’
    RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
                         ^
.../drivers/net/nfp/nfp_net.c:2978:2: note: in expansion of macro ‘PMD_CPP_LOG’
  PMD_CPP_LOG(DEBUG, "%s: offset size %lu, count_size: %lu\n", __func__,
  ^~~~~~~~~~~
>  
> 
>     Thanks,
>     ferruh
>