Re: [dpdk-dev] [PATCH v2] kni: use kni_ethtool_ops only with unknown drivers

[Igor Ryzhov]

Hi Ferruh,

Thank you for all your comments.

The only real purpose of the patch is to support .get_link for all KNI
interfaces, not just for those using igb or ixgbe.
So I propose to remove ethtool support at all with your patch, and after
that, I will add .get_link support again.

Yes, I understand that it doesn't implement real link status support, it
just relies on the application, that should set link status using sysfs.

Regarding the ethtool support in a driver-independent way – the idea is to
use the same req/resp queues
which are used for some net_device_ops like .ndo_change_mtu or
.ndo_set_mac_address.
I want to add more types of requests to rte_kni_req_id enum and implement
them.
It's not an urgent task, so I don't know yet when I'll find time to do it.

Best regards,
Igor

On Tue, Dec 18, 2018 at 9:13 PM Ferruh Yigit  wrote:

> On 12/3/2018 2:06 PM, Igor Ryzhov wrote:
> > Hi Ferruh,
> >
> > What about the patch?
> >
> > I also support dropping ethtool for ixgbe and i40e, but to save generic
> ethtool_ops
> > with .get_link implementation, because it's an essential function that
> works
> > correctly
> > after proper implementation of carrier status that was merged into 18.11.
> >
> > Also, other ethtool operations may be implemented in a
> driver-independent way using
> > the same concept as for netdev_ops.
>
> "carrier status" relies on the sample app support also relies on kni
> net_device
> sysfs interface.
> It is good target to have ethtool support in a driver-independent way,
> please
> share more details and lets discuss them.
>
> >
> > On Mon, Dec 3, 2018 at 4:09 PM Ferruh Yigit  > > wrote:
> >
> > On 11/30/2018 11:38 PM, Stephen Hemminger wrote:
> > > On Fri, 30 Nov 2018 22:47:50 +0300
> > > Igor Ryzhov mailto:iryz...@nfware.com>>
> wrote:
> > >
> > >> Current implementation of kni_ethtool_ops just uses corresponding
> > >> ethtool_ops function of underlying driver for all functions
> except for
> > >> .get_link. This commit sets kni->net_dev->ethtool_ops directly to
> the
> > >> ethtool_ops of the corresponding driver.
> > >>
> > >> For unknown drivers (all but ixgbe and i40e) we still use
> > >> kni_ethtool_ops with implemented .get_link function.
> > >>
> > >> Signed-off-by: Igor Ryzhov  iryz...@nfware.com>>
> > >
> > > Why does KNI still support ethtool which:
> > >   1. Only works on a subset of devices
> > >   2. Requires a 3rd implmentation of the HW device (Linux, DPDK,
> and KNI)
> >
> > +1 to drop ethtool support, last time we tried concern was anybody
> may be using
> > it, perhaps we can try again.
> >
> > >
> > > Then again why does KNI exist at all? What is missing from virtio
> user which
> > > is faster anyway.
> > >
> >
>
>

Re: [dpdk-dev] [RFC] kni: remove ethtool support

[Igor Ryzhov]

Hi Ferruh,

I answered in another thread.

Regarding this patch – I have no objections now.

Best regards,
Igor

On Tue, Dec 18, 2018 at 9:17 PM Ferruh Yigit  wrote:

> On 12/18/2018 9:20 AM, Ferruh Yigit wrote:
> > On 12/18/2018 8:20 AM, Igor Ryzhov wrote:
> >> Hi Ferruh,
> >>
> >> Please, look at my patch http://patches.dpdk.org/patch/48454/ and
> consider
> >> rebasing your patch over mine.
> >
> > Sorry about that, yes I will check it today.
>
> Hi Igor,
>
> I put some comments on your patch.
>
> As far as I can see it also has a target to remove current type of ethtool
> support, so this RFC should not be a concern to you.
> All ethtool support can be removed, when you have an actual solution for
> driver
> independent ethtool support only a little code needs to be added back.
>
> Thanks,
> ferruh
>
> >
> >>
> >> As we discussed with Stephen, KNI needs to supply ethtool_ops with
> >> .get_link function, to properly support link status.
> >> So we should save ethtool_ops and implement .get_link using standard
> >> ethtool_op_get_link.
> >>
> >> Best regards,
> >> Igor
> >
> >
>
>

Re: [dpdk-dev] [EXT] Default cacheline size for ARM

[Honnappa Nagarahalli]

> 
> On Fri, 2019-01-04 at 19:59 +, Yongseok Koh wrote:
> > ---
> > ---
> > Hi,
> >
> > The cacheline size (RTE_CACHE_LINE_SIZE) for ARM CPUs is set to be
> > 128B by default. Mellanox's BlueField is an ARM CPU having Cortex-A72
> > and its CL size is 64B.
Just wondering how many devices are out there with 128B cache line? I also have 
not heard about any future devices with 128B cache line. If the majority is 
64B, why not keep 64B as the default?

> >
> > I can add config/defconfig_arm64-bluefield-linuxapp-gcc for legacy
> > build anyway.
> >
> 
> Makes sense.
> 
> > For the meson build, I know it parses the Main ID register to figure
> > out Implementor ID and Part Number. However, Mellanox doesn't program
> > our own ID yet but we set the Part Number as 0xd08 (A72).
> > According to my folks, ARM's A53, A57, A72, and A73 designs all have
> > 64B CL. If that's true, can I push a patch to make the change?
> 
> Yes. Broadcom Stingray has the same situation i.e Use flags_generic,
> machine_args_generic
> 
This will solve only the native compilation issue. It will not address distro 
compilation (may be this is not your goal?).

> >
> > Please comment.
> >
> >
> > Thanks,
> > Yongseok
> >

Re: [dpdk-dev] [PATCH v4 0/3] support flow counters using devx

[Shahaf Shuler]

Thursday, January 3, 2019 5:07 PM, Mordechay Haimovsky:
> Subject: [dpdk-dev] [PATCH v4 0/3] support flow counters using devx
> 
> This series of commits add support for creating, allocating, querying and
> destroying flow counters in mlx5 PMD using the devx interface.
> 
> Moti Haimovsky (3):
>   net/mlx5: fix shared counter allocation logic
>   net/mlx5: add devx functions to glue
>   net/mlx5: support flow counters using devx
> 
>  drivers/net/mlx5/Makefile  |  11 ++
>  drivers/net/mlx5/meson.build   |   5 +
>  drivers/net/mlx5/mlx5.c|  16 ++-
>  drivers/net/mlx5/mlx5.h|  15 +++
>  drivers/net/mlx5/mlx5_devx_cmds.c  | 107 +
>  drivers/net/mlx5/mlx5_flow.h   |  10 +-
>  drivers/net/mlx5/mlx5_flow_dv.c| 234
> +++--
>  drivers/net/mlx5/mlx5_flow_verbs.c |  14 +--
>  drivers/net/mlx5/mlx5_glue.c   |  98 
>  drivers/net/mlx5/mlx5_glue.h   |  19 +++
>  drivers/net/mlx5/mlx5_prm.h|  71 +++
>  11 files changed, 578 insertions(+), 22 deletions(-)  create mode 100644
> drivers/net/mlx5/mlx5_devx_cmds.c

Series applied to next-net-mlx, thanks.

Moti - please notice your author name and singed-of name are not aligned. 
I fixed it for this series, please update your gitconfig for them to match on 
the next patches. 

> 
> --
> 1.8.3.1

Re: [dpdk-dev] [EXT] Default cacheline size for ARM

[Jerin Jacob Kollanukkaran]

On Sat, 2019-01-05 at 22:47 +, Honnappa Nagarahalli wrote:
> > On Fri, 2019-01-04 at 19:59 +, Yongseok Koh wrote:
> > > ---
> > > 
> > > ---
> > > Hi,
> > > 
> > > The cacheline size (RTE_CACHE_LINE_SIZE) for ARM CPUs is set to
> > > be
> > > 128B by default. Mellanox's BlueField is an ARM CPU having
> > > Cortex-A72
> > > and its CL size is 64B.
> Just wondering how many devices are out there with 128B cache line? I
> also have not heard about any future devices with 128B cache line. If
> the majority is 64B, why not keep 64B as the default?

The problem is, In the armv8 spec the cache line size is IMPLEMENTATION
DEFINED. Marvell's embedded processors has 128B CL and Server
processors has 64B CL.

Assuming the defconfig_arm64-armv8a-linuxapp-gcc will be used by distro
folks then that configuration should support all the devices with
armv8.1 spec. For instance, marvells new chips are armv8.2 but we can
not make that as default.

Target specific configuration can be used for optimized target specific
configuration.

Or any other ideas for distro config.

> 
> > > I can add config/defconfig_arm64-bluefield-linuxapp-gcc for
> > > legacy
> > > build anyway.
> > > 
> > 
> > Makes sense.
> > 
> > > For the meson build, I know it parses the Main ID register to
> > > figure
> > > out Implementor ID and Part Number. However, Mellanox doesn't
> > > program
> > > our own ID yet but we set the Part Number as 0xd08 (A72).
> > > According to my folks, ARM's A53, A57, A72, and A73 designs all
> > > have
> > > 64B CL. If that's true, can I push a patch to make the change?
> > 
> > Yes. Broadcom Stingray has the same situation i.e Use
> > flags_generic,
> > machine_args_generic
> > 
> This will solve only the native compilation issue. It will not
> address distro compilation (may be this is not your goal?).

See above.

> 
> > > Please comment.
> > > 
> > > 
> > > Thanks,
> > > Yongseok
> > >