Re: [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/27/2017 12:21 PM, Bruce Richardson wrote:
> On Tue, Jun 27, 2017 at 12:08:56PM +0100, Ferruh Yigit wrote:
>> On 6/26/2017 10:42 AM, Radu Nicolau wrote:
>>> From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
>>>
>>> If-MIB xstats:
>>> ifNumber
>>> ifIndex
>>> ifType
>>> ifMtu
>>> ifSpeed
>>> ifPhysAddress
>>> ifOperStatus
>>> ifLastChange
>>> ifHighSpeed
>>> ifConnectorPresent
>>> ifCounterDiscontinuityTime
>>>
>>> EtherLike-MIB xstats:
>>> dot3PauseOperMode
>>> dot3StatsDuplexStatus
>>> dot3StatsRateControlAbility
>>> dot3StatsRateControlStatus
>>> dot3ControlFunctionsSupported
>>>
>>> -updated in v2: coding style
>>>
>>> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
>>> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>
>> <...>
>>
>> This patch implements MIBs for some Intel drivers using xstats, this is
>> easy way to get some information from PMDs.
>>
>> But there was a outstanding comment to make this ethdev layer.
>>
>> So I believe we have two options:
>>
>> [1]
>> Each PMD implements MIBs using xstats, this is pragmatic solution and
>> each PMD can implement whichever MIBs they want.
>>
>> [2]
>> Implement a ethdev layer API and add a new dev_ops to get MIBs, API can
>> use existing methods to get required information, and if it fails can
>> call dev_ops which can be similar to the xstats.
>> Because of API all PMDs can have a small amount of support by default
>> and they can implement dev_ops for more support.
>>
>>
>> Although 2) looks more generic and proper, I am not really sure if that
>> is overkill and if this worth to the effort and to have new API and
>> dev_ops, comparing current method is easy to implement, any comments?
>>
> 
> For 2, does the "use existing methods" include calling xstats? If so,
> then we can just drop the requirement for 2 to have any new functions
> implemented in the driver. Instead, have the information provided by
> drivers in the normal xstats call, but have a new ethdev API to provide
> that information to the app - basically hiding the xstats complexity
> underneath.

I was thinking another dev_ops similar to xstats, but re-using xstats
can be better idea here.

so option 2 becomes:
[2] An eth_dev API to use existing APIs to get information from PMDs and
wrap MIBs related xstats calls.

But I believe if option 2 is overkill for this task question is still valid.

Thanks,
ferruh