Re: [PATCH 2/2] ethtool: add new library to provide ethtool-alike APIs

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-06-05 11:25, Wang, Liang-min:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Stephen and me say the same thing about using the ethdev API.
> > We don't understand why using a fake ethtool lib would be easier.
> > Though you are saying it "facilitated [your] adoption to dpdk".
> > Please could you explain why using an ethtool-like API is easier than using
> > the existing ethdev API?
> > In any case, you have to develop a specific backend for DPDK (rte_ethtool
> > would be also DPDK-specific).
> 
> As described earlier in this patch comment reply, there are other ethtool ops that have been implemented.
> Those ops includes set/get eeprom, set/get pauseparam, set/get ringparam which are not available in the exiting ethdev library.

1/ We cannot really consider code which is not public
2/ You may extend ethdev if some functions are missing

> For this release, we focus on releasing some basic functions (btw, mac_addr_set is not available but is covered by this patch).

Yes, you are extending ethdev by adding rte_eth_dev_default_mac_addr_set.

> The key reason that this set of library is not released as part of ethdev is the ethtool API dependency on kernel include file.

It is a good reason to separate the library.
But it doesn't justify its need.

> To faithfully carry the ethtool ops and net dev ops API parameters, the ethtool APIs are designed to follow the original definition except avoiding carry kernel states.
> With that, to support ethtool APIs faithfully, we need to include <linux/ethtool.h>. 
> As suggested by many DPDK veterans including Thomas (indicated over your reply), you would prefer these APIs in a separate library.

I think I'm starting to understand that you really need ethtool conversion
(implemented in rte_ethtool_get_drvinfo) but not the other functions which
are simple wrappers. Right?