>2015-05-29 15:26, Liang-Min Larry Wang: >> adding a new library based upon ethdev APIs to provide API's that bear >> the same functionality as ethtool_ops (linux/ethtool.h) and >> net_device_ops (linux/netdevice.h). >> >> Signed-off-by: Liang-Min Larry Wang <liang-min.wang at intel.com> >> --- >> MAINTAINERS | 4 + >> config/common_linuxapp | 5 + >> lib/Makefile | 1 + >> lib/librte_ethtool/Makefile | 56 +++++++ >> lib/librte_ethtool/rte_ethtool.c | 155 +++++++++++++++++ >> lib/librte_ethtool/rte_ethtool.h | 257 >> +++++++++++++++++++++++++++++ >> lib/librte_ethtool/rte_ethtool_version.map | 18 ++ >> mk/rte.app.mk | 1 + > >NACK for several reasons: >- It's unclear what benefits this ethdev wrapper is bringing
Since ethtool is provided to assist users migrating from kernel ethtool/net_device_op based design to user-space DPDK device management. The ethtool API's are created to closely maintain its original interface, therefore this library depends on <linux/ethool.h>. To avoid pollute the existing ethdev interface, a new library is created. To minimize code replication and maintain closely 1:1 API definition with kernel space API, this interface is designed based upon available ethdev APIs and add additional dev_ops if it's necessary. >- There is no obvious interest (how is it supposed to be used?) There are already two acknowledge on this release. Earlier comment on this patch has that " ... The API's for ethtool like things are valuable ..." >- There is no update in the doc/ directory Need more guidance on that. >Other comments: >- the patches are not versioned There is version file. Not sure what do you mean "the patches are not versioned" >- the copyright starts in 2010 Will fix that. >I'm curious to understand how renaming rte_eth_dev_set_mtu() to >rte_ethtool_net_change_mtu() will help anyone. As described, this interface is designed to provide API closely to kernel space ethtool ops and net_device_op.
