[dpdk-dev] [PATCH v4 7/7] ethdev: hide eth dev related structures

[Ananyev, Konstantin]

> 
> On Tue, Oct 5, 2021 at 12:43 PM Ferruh Yigit <ferruh.yigit at intel.com> wrote:
> > > This change is going to hurt a lot of people :-).
> > > But this is a necessary move.
> > >
> >
> > +1 that it is necessary move, but I am surprised to see how much 'rte_eth_devices'
> > is accessed directly.
> >
> > Do you have any idea/suggestion on how can we reduce the pain for them?
> 
> From what I see, ethdev iterators are probably something that is not
> known enough.
> rte_eth_dev_info_get() also fills some other spots.
> I don't have a magic answer, people need to look at existing API.
> 
> 
> But I just scratched the surface, looking at rte_eth_devices[] accesses.
> There might be other rte_eth_dev object dereferences (you get one
> point calling rte_eth_dev_info_get) that my grep did not catch.
> 

Indeed, that's a quite lot...
Thanks a lot for such detailed list, very interesting.
Wonder should we heads up coming changes to these guys...
Or might be interested persons are already aware (by reading dev at dpdk.org or so).

> Details:
> 
> >
> > > $ git grep-all -lw rte_eth_devices |grep -v \\.patch$
> > > ANS/ans/ans_main.c
> 
> I think this code is just lagging behind what ethdev currently
> provides/does wrt default offload config.
> This code probably does not need to dereference rte_eth_devices[] to
> query offloads configuration.
> 
> 
> > > BESS/core/drivers/pmd.cc
> 
> ethdev iterators can replace those accesses.
> 
> 
> > > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/qdma_xdebug.c
> > > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/rte_pmd_qdma.c
> > > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/pcierw.c
> > > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/testapp.c
> 
> This is a DPDK clone, with an additional driver, so irrelevant.
> 
> 
> > > FD.io-VPP/src/plugins/dpdk/device/format.c
> 
> rte_eth_rx_burst_mode_get() and rte_eth_tx_burst_mode_get() should do the job.
> I wonder if those APIs were introduced in DPDK for VPP.. ?
> 
> 
> > > lagopus/src/dataplane/dpdk/dpdk_io.c
> 
> Idem, ethdev iterators and rte_eth_dev_info_get() instead of direct
> access for dev_flags.
> 
> 
> > > OVS/lib/netdev-dpdk.c
> 
> For OVS, it was ethdev iterators + rte_eth_dev_info_get() where necessary.
> 
> 
> > > packet-journey/app/kni.c
> 
> There might be something missing in current ethdev API.
> This app wants to know if device is started... but on the other hand,
> that's probably something the app tracks itself.
> 
> 
> > > pktgen-dpdk/app/pktgen-port-cfg.c
> > > pktgen-dpdk/app/pktgen-port-cfg.h
> > > pktgen-dpdk/app/pktgen-stats.c
> 
> Accesses to offload configuration which I think are unneeded (like ANS).
> Direct access for dev_flags, can be replaced with rte_eth_dev_info_get.
> Direct access for name, can be replaced with rte_eth_dev_info_get.
> 
> 
> > > Trex/src/dpdk_funcs.c
> > > Trex/src/drivers/trex_i40e_fdir.c
> > > Trex/src/drivers/trex_ixgbe_fdir.c
> 
> Here, there is some horror.
> Directly casting and accessing hardware:
>     struct rte_eth_dev *dev = &rte_eth_devices[repid];
>         struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>         I40E_WRITE_REG(hw, I40E_GLQF_ORT(12), 0x00000062);
>         I40E_WRITE_REG(hw, I40E_GLQF_PIT(2), 0x000024A0);
>         I40E_WRITE_REG(hw,
> I40E_PRTQF_FD_INSET(I40E_FILTER_PCTYPE_NONF_IPV4_UDP, 0), 0);
> etc...
> 
> This code probably bypasses too much of dpdk API, I stopped at this.
> 
> 
> > > TungstenFabric-vRouter/gdb/vr_dpdk.gdb
> 
> Mm, interesting, this part displays DPDK internals from gdb.
> That's something I have in my todolist for a long time, providing some
> common gdb scripts in DPDK...
> 
> 
> --
> David Marchand