Re: no printf in drivers code

[Niklas Soderlund]

Hi Thomas,

On 2023-02-03 14:57:06 +0100, Thomas Monjalon wrote:
> Hello,
> 
> We have too many drivers using printf() instead of rte_log().
> Please propose a plan to replace or remove the calls to printf().
> If no plan, I could propose one but you may not like it :)
> 
> Affected drivers are:
>   baseband/acc
>   bus/dpaa
>   bus/fslmc
>   crypto/caam_jr
>   crypto/ccp
>   dma/ioat
>   net/dpaa
>   net/dpaa2
>   net/nfp

A patch for NFP have been post as part of the series '[PATCH 0/3] 
cleanup the PMD'.

>   net/qede
>   net/txgbe
>   raw/ifpga
> 
> PS: printf is allowed for tests and dumps.
> 
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH] doc: add supported APIs section to nfp guide

[Niklas Soderlund]

Hello Ajit,

On 2023-01-24 09:13:20 +, Ferruh Yigit wrote:
> On 1/24/2023 2:44 AM, Chaoyong He wrote:
> >> On 12/1/2022 1:38 AM, Chaoyong He wrote:
> >>> Add a new section 'Supported APIs', inculding the supported
> >>> APIs/items/actions of rte_flow.
> >>>
> >>
> >> Isn't this information already exists in 
> >> 'doc/guides/nics/features/nfp.ini'? Why to
> >> duplicate here?
> >>
> > 
> > Oh, one customer say they wants this section like the `bnxt.rst`, so we 
> > added this.
> > 
> 
> You are right, and I think that is a duplication for bnxt too, cc'ed
> Ajit for comment.

Small ping on a comment here.

> 
> 
> >>> Signed-off-by: Chaoyong He 
> >>> Reviewed-by: Niklas Söderlund 
> >>> ---
> >>>  doc/guides/nics/nfp.rst | 71
> >>> +
> >>>  1 file changed, 71 insertions(+)
> >>>
> >>> diff --git a/doc/guides/nics/nfp.rst b/doc/guides/nics/nfp.rst index
> >>> b74067c875..615f7dab9f 100644
> >>> --- a/doc/guides/nics/nfp.rst
> >>> +++ b/doc/guides/nics/nfp.rst
> >>> @@ -213,3 +213,74 @@ PF vNIC.
> >>>  The ctrl vNIC service handling various control message, like the
> >>> creation and  configuration of representor port, the pattern and
> >>> action of flow rules, the  statistics of flow rules, and so on.
> >>> +
> >>> +Supported APIs
> >>> +--
> >>> +
> >>> +rte_flow APIs
> >>> +~
> >>> +
> >>> +Listed below are the rte_flow functions supported:
> >>> +* ``rte_flow_ops_get``
> >>> +* ``rte_flow_validate``
> >>> +* ``rte_flow_create``
> >>> +* ``rte_flow_destroy``
> >>> +* ``rte_flow_flush``
> >>> +* ``nfp_flow_query``
> >>> +* ``rte_flow_tunnel_action_decap_release``
> >>> +* ``rte_flow_tunnel_decap_set``
> >>> +* ``rte_flow_tunnel_item_release``
> >>> +* ``rte_flow_tunnel_match``
> >>> +
> >>> +rte_flow Items
> >>> +~~
> >>> +
> >>> +Refer to "Table 1.2 rte_flow items availability in networking
> >>> +drivers" in `Overview of Networking Drivers
> >> `.
> >>> +
> >>> +Listed below are the rte_flow items supported:
> >>> +
> >>> +* ``eth``
> >>> +* ``geneve``
> >>> +* ``gre``
> >>> +* ``gre_key``
> >>> +* ``ipv4``
> >>> +* ``ipv6``
> >>> +* ``port_id``
> >>> +* ``sctp``
> >>> +* ``tcp``
> >>> +* ``udp``
> >>> +* ``vlan``
> >>> +* ``vxlan``
> >>> +
> >>> +rte_flow Actions
> >>> +
> >>> +
> >>> +Refer to "Table 1.3 rte_flow actions availability in networking
> >>> +drivers" in `Overview of Networking Drivers
> >> `.
> >>> +
> >>> +Listed below are the rte_flow actions supported:
> >>> +
> >>> +* ``count``
> >>> +* ``drop``
> >>> +* ``jump``
> >>> +* ``of_pop_vlan``
> >>> +* ``of_push_vlan``
> >>> +* ``of_set_vlan_pcp``
> >>> +* ``of_set_vlan_vid``
> >>> +* ``raw_decap``
> >>> +* ``raw_encap``
> >>> +* ``port_id``
> >>> +* ``set_ipv4_dscp``
> >>> +* ``set_ipv4_dst``
> >>> +* ``set_ipv4_src``
> >>> +* ``set_ipv6_dscp``
> >>> +* ``set_ipv6_dst``
> >>> +* ``set_ipv6_src``
> >>> +* ``set_mac_dst``
> >>> +* ``set_mac_src``
> >>> +* ``set_tp_dst``
> >>> +* ``set_tp_src``
> >>> +* ``set_ttl``
> >>> +* ``vxlan_decap``
> >>> +* ``vxlan_encap``
> > 
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk

[Niklas Soderlund]

Hello again,

On 2023-02-16 11:41:13 +0100, Chaoyong He wrote:
> 
> 
> > -Original Message-
> > From: Niklas Soderlund 
> > Sent: Thursday, February 16, 2023 6:37 PM
> > To: Kevin Traynor 
> > Cc: Ferruh Yigit ; Xueming(Steven) Li
> > ; Chaoyong He ;
> > dev@dpdk.org; Luca Boccassi ; oss-drivers  > driv...@corigine.com>; Nole Zhang ; Kevin Liu
> > ; sta...@dpdk.org
> > Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
> > NFDk
> > 
> > Hi Kevin,
> > 
> > Thanks for your input.
> > 
> > On 2023-02-16 10:28:34 +, Kevin Traynor wrote:
> > > On 15/02/2023 18:28, Ferruh Yigit wrote:
> > > > On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> > > > > Hi Ferruh,
> > > > >
> > > > > Thanks for your continues effort in dealing with NFP patches.
> > > > >
> > > > > On 2023-02-15 13:42:01 +, Ferruh Yigit wrote:
> > > > > > On 2/8/2023 9:15 AM, Chaoyong He wrote:
> > > > > > > From: Peng Zhang 
> > > > > > >
> > > > > > > 48-bit DMA address is supported in the firmware with NFDk, so
> > > > > > > enable this feature in PMD now. But the firmware with NFD3
> > > > > > > still just support 40-bit DMA address.
> > > > > > >
> > > > > > > RX free list descriptor, used by both NFD3 and NFDk, is also
> > > > > > > modified to support 48-bit DMA address. That's OK because the
> > > > > > > top bits is always set to 0 when assigned with 40-bit DMA address.
> > > > > > >
> > > > > > > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> > > > > > > Cc: jin@corigine.com
> > > > > > > Cc: sta...@dpdk.org
> > > > > > >
> > > > > >
> > > > > > Why a backport is requested? As far as I understand this is not
> > > > > > fixing anything but extending device capability. Is this a fix?
> > > > >
> > > > > I agree this is a bit of a grey zone. We reasoned this was a fix
> > > > > as we should have done this from the start in the commit that
> > > > > added support for NFDk. Are you OK moving forward with this as a
> > > > > fix or would you prefer we resubmit without the request to backport?
> > > > >
> > > >
> > > > I am not sure, is this change have any potential to change behavior
> > > > for existing users?
> > > > Like if one of your user is using 22.11.1 release, and if this patch
> > > > backported to next LTS version, 22.11.2, will user notice any 
> > > > difference?
> > > >
> > > >
> > > > @Luca, @Kevin, what is your comment as LTS maintainers?
> > > >
> > >
> > > A bit difficult to know. If NFDk is not practicably usable without it,
> > > then it could be considered a fix. If it's just extending to add
> > > nice-to-have functionality then probably it is not a fix.
> > 
> > I think we can treat this as a nice-to-have and not something that makes
> > NFDk unusable. As stated above, we marked this as a Fix as we *really*
> > should have done this in the commit which added NFDk support.
> > 
> > @Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC
> > tags when/if applying?
> > 
> 
> Actually, the DPDK app using the nfp card with a firmware of NFDk will 
> coredump without this patch.
> And that's the directly reason we consider backport this patch.

Wops, you are right I was thinking of a different patch when replying.  
Sorry about the confusion.

Yes this is a bug fix that should be back ported. Thanks Chaoyong for 
correcting me.

> 
> > >
> > > It would need to ensure that it is tested on 22.11 branch and there
> > > are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
> > > Xueming who will ultimately decide.
> > >
> > > A guide below on some things to consider for this type of backport is 
> > > here:
> > > http://doc.dpdk.org/guides/contributing/stable.html#what-changes-shoul
> > > d-be-backported
> > >
> > > > > >
> > > > > > > Signed-off-by: Peng Zhang 
> > > > > > > Reviewed-by: Chaoyong He 
> > > > > > > Reviewed-by: Niklas Söderlund 
> > > > > >
> > > > >
> > > >
> > >
> > 
> > --
> > Kind Regards,
> > Niklas Söderlund

-- 
Kind Regards,
Niklas Söderlund

Re: FW: [PATCH v1] buildtools: ensure the NUMA nodes are counted correct

[Niklas Soderlund]

Hi Thomas,

Have you checked if this address the same issue you where seeing? Do you 
think we can move forward with this fix?

On 2022-08-31 10:47:24 +0200, Nole Zhang wrote:
> 
> 
> 
> > -Original Message-
> > From: Thomas Monjalon 
> > Sent: 2022年8月29日 21:15
> > To: Nole Zhang ; Chaoyong He 
> > 
> > Subject: Re: [PATCH v1] buildtools: ensure the NUMA nodes are counted 
> > correct
> > 
> > 29/08/2022 13:17, Nole Zhang:
> > > From: Thomas Monjalon 
> > > > 02/08/2022 09:54, Chaoyong He:
> > > > > From: Peng Zhang 
> > > > >
> > > > > Sorting a list of strings with the format "node[0-9]+" in order 
> > > > > to find the largest integer by looking at the last item after 
> > > > > the sort breaks. But if there are more then 10 items as a string 
> > > > > sort will sort "node10" before "node2", it will get the error NUMA 
> > > > > nodes.
> > > >
> > > > What is the error you are seeing?
> > > >
> > > >
> > > We get the error NUMA, in this example, we get the NUMA nodes is 10, 
> > > But at fact, it has 11 NUMA.
> > 
> > Please give more details, where do you see this error?
> > We should know how to reproduce and check we have the same issue.
> > Thanks
> > 
> > Please reply with a detailed answer on the mailing list.
> > 
> In the China Phytium S2500 CPU + INSPUR server, it has 16 NUMA.
> The details are as follows:
> 
> ~#: lscpu
> 
> Architecture:aarch64
> CPU op-mode(s):  64-bit
> Byte Order:  Little Endian
> CPU(s):  128
> On-line CPU(s) list: 0-127
> Thread(s) per core:  1
> Core(s) per socket:  64
> Socket(s):   2
> NUMA node(s):16
> Vendor ID:   0x70
> Model:   3
> Model name:  S2500
> Stepping:0x1
> BogoMIPS:100.00
> L1d cache:   4 MiB
> L1i cache:   4 MiB
> L2 cache:64 MiB
> L3 cache:128 MiB
> NUMA node0 CPU(s):   0-7
> NUMA node1 CPU(s):   8-15
> NUMA node2 CPU(s):   16-23
> NUMA node3 CPU(s):   24-31
> NUMA node4 CPU(s):   32-39
> NUMA node5 CPU(s):   40-47
> NUMA node6 CPU(s):   48-55
> NUMA node7 CPU(s):   56-63
> NUMA node8 CPU(s):   64-71
> NUMA node9 CPU(s):   72-79
> NUMA node10 CPU(s):  80-87
> NUMA node11 CPU(s):  88-95
> NUMA node12 CPU(s):  96-103
> NUMA node13 CPU(s):  104-111
> NUMA node14 CPU(s):  112-119
> NUMA node15 CPU(s):  120-127
> Flags:   half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva 
> idivt lpae evtstrm
> 
> 
> I use  meson build -Dmax_lcores=detect -Dmax_numa_nodes=detect to compile, 
> then dpdk initialization only shows 10 numa. 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 13/20] remove repeated word 'that'

[Niklas Soderlund]

Hi Stephen,

Thanks for your work.

On 2022-07-25 20:34:56 -0700, Stephen Hemminger wrote:
> Found by doing duplicate word scan.
> 
> Signed-off-by: Stephen Hemminger 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/net/nfp/nfp_ctrl.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h
> index 372d53746243..2327d4eb7646 100644
> --- a/drivers/net/nfp/nfp_ctrl.h
> +++ b/drivers/net/nfp/nfp_ctrl.h
> @@ -182,7 +182,7 @@
>   * Reuse spare address to contain the offset from the start of
>   * the host buffer where the first byte of the received frame
>   * will land.  Any metadata will come prior to that offset.  If the
> - * value in this field is 0, it means that that the metadata will
> + * value in this field is 0, it means that the metadata will
>   * always land starting at the first byte of the host buffer and
>   * packet data will immediately follow the metadata.  As always,
>   * the RX descriptor indicates the presence or absence of metadata
> -- 
> 2.35.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH] net/nfp: remove unneeded header inclusion

[Niklas Soderlund]

Hi David,

Thanks for your work.

On 2022-04-08 11:41:16 +0200, David Marchand wrote:
> Looking at this driver history, there was never a need for including
> execinfo.h.
> 
> Signed-off-by: David Marchand 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c 
> b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> index bad80a5a1c..08bc4e8ef2 100644
> --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> @@ -16,9 +16,6 @@
>  
>  #include 
>  #include 
> -#if defined(RTE_BACKTRACE)
> -#include 
> -#endif
>  #include 
>  #include 
>  #include 
> -- 
> 2.23.0
> 

-- 
Kind Regards,
Niklas Söderlund