From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Duszynski Subject: Re: [PATCH v3 2/4] net/mrvl: add mrvl net pmd driver Date: Wed, 4 Oct 2017 10:59:59 +0200 Message-ID: <20171004085959.GB5668@tdu> References: <1506594158-15721-2-git-send-email-tdu@semihalf.com> <1507031500-11473-1-git-send-email-tdu@semihalf.com> <1507031500-11473-3-git-send-email-tdu@semihalf.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Tomasz Duszynski , dev@dpdk.org, mw@semihalf.com, dima@marvell.com, nsamsono@marvell.com, Jianbo.liu@linaro.org, Jacek Siuda To: Ferruh Yigit Return-path: Received: from mail-lf0-f43.google.com (mail-lf0-f43.google.com [209.85.215.43]) by dpdk.org (Postfix) with ESMTP id 44D197CBD for ; Wed, 4 Oct 2017 11:00:02 +0200 (CEST) Received: by mail-lf0-f43.google.com with SMTP id b127so12574669lfe.9 for ; Wed, 04 Oct 2017 02:00:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: 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 Wed, Oct 04, 2017 at 01:24:27AM +0100, Ferruh Yigit wrote: > On 10/3/2017 12:51 PM, Tomasz Duszynski wrote: > > Add support for the Marvell PPv2 (Packet Processor v2) 1/10 Gbps adapte= r. > > Driver is based on external, publicly available, light-weight Marvell > > MUSDK library that provides access to network packet processor. > > > > Driver comes with support for the following features: > > > > * Speed capabilities > > * Link status > > * Queue start/stop > > * MTU update > > * Jumbo frame > > * Promiscuous mode > > * Allmulticast mode > > * Unicast MAC filter > > * Multicast MAC filter > > * RSS hash > > * VLAN filter > > * CRC offload > > * L3 checksum offload > > * L4 checksum offload > > * Packet type parsing > > * Basic stats > > * Stats per queue > > I have more detailed comments but in high level, > what do you think splitting this patch into three patches: > - Skeleton > - Add Rx/Tx support > - Add features, like MTU update or Promiscuous etc.. support If it's how submission process works then I think you left me with no other option than splitting driver into nice patchset :). On the other hand driver is really a wrapper to MUSDK library and thus quite easy to follow. What are the benefits of such 3-way split? > > > > > Driver was engineered cooperatively by Semihalf and Marvell teams. > > > > Semihalf: > > Jacek Siuda > > Tomasz Duszynski > > > > Marvell: > > Dmitri Epshtein > > Natalie Samsonov > > > > Signed-off-by: Jacek Siuda > > Signed-off-by: Tomasz Duszynski > > <...> > > > +static struct rte_vdev_driver pmd_mrvl_drv =3D { > > + .probe =3D rte_pmd_mrvl_probe, > > + .remove =3D rte_pmd_mrvl_remove, > > +}; > > + > > +RTE_PMD_REGISTER_VDEV(net_mrvl, pmd_mrvl_drv); > > Please help me understand. > > This driver implemented as virtual driver, because: > With the help of custom kernel modules, musdk library already provides > userspace datapath support. This PMD is an interface to musdk library. > Is this correct? That is right. Another reason this NIC is not PCI device. > > If so, just thinking loud: > - Why not implement this PMD directly on top of kernel interface, > removing musdk layer completely? > - How big problem that this PMD depends on custom kernel code? I think the main reason is that MUSDK is already used in different projects. Keeping multiple codebases offering similar functionality would be quite demanding in terms of extra work needed. > - How library and custom kernel code delivered? For which platforms? Kernel and library sources are hosted on publicly available repository. Driver was tested on Armada 7k/8k SoCs. > > <....> > -- - Tomasz Duszy=C5=84ski