Re: [PATCH v3 2/4] net/mrvl: add mrvl net pmd driver

[Tomasz Duszynski <tdu@semihalf.com>]

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 adapter.
> > 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 <jck@semihalf.com>
> > Tomasz Duszynski <tdu@semihalf.com>
> >
> > Marvell:
> > Dmitri Epshtein <dima@marvell.com>
> > Natalie Samsonov <nsamsono@marvell.com>
> >
> > Signed-off-by: Jacek Siuda <jck@semihalf.com>
> > Signed-off-by: Tomasz Duszynski <tdu@semihalf.com>
>
> <...>
>
> > +static struct rte_vdev_driver pmd_mrvl_drv = {
> > +	.probe = rte_pmd_mrvl_probe,
> > +	.remove = 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ński