netdev - Re: [PATCH net-next v2] drivers, ixgbe: show VF statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 May 2022 09:20:14 -0700
From:   Tony Nguyen <anthony.l.nguyen@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Maximilian Heyne <mheyne@...zon.de>
CC:     Jesse Brandeburg <jesse.brandeburg@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2] drivers, ixgbe: show VF statistics



On 5/6/2022 9:13 AM, Jakub Kicinski wrote:
> On Fri, 6 May 2022 06:44:40 +0000 Maximilian Heyne wrote:
>> On 2022-05-04T20:16:32-07:00   Jakub Kicinski <kuba@...nel.org> wrote:
>>
>>> On Tue, 3 May 2022 15:00:17 +0000 Maximilian Heyne wrote:
>>>> +		for (i = 0; i < adapter->num_vfs; i++) {
>>>> +			ethtool_sprintf(&p, "VF %u Rx Packets", i);
>>>> +			ethtool_sprintf(&p, "VF %u Rx Bytes", i);
>>>> +			ethtool_sprintf(&p, "VF %u Tx Packets", i);
>>>> +			ethtool_sprintf(&p, "VF %u Tx Bytes", i);
>>>> +			ethtool_sprintf(&p, "VF %u MC Packets", i);
>>>> +		}
>>>
>>> Please drop the ethtool stats. We've been trying to avoid duplicating
>>> the same stats in ethtool and iproute2 for a while now.
>>
>> I can see the point that standard metrics should only be reported via the
>> iproute2 interface. However, in this special case this patch was
>> intended to converge the out-of-tree driver with the in-tree version.
>> These missing stats were breaking our userspace. If we now switch
>> solely to iproute2 based statistics both driver versions would
>> diverge even more. So depending on where a user gets the ixgbe driver
>> from, they have to work-around.
>>
>> I can change the patch as requested, but it will contradict the inital
>> intention. At least Intel should then port this change to the
>> external driver as well. Let's get a statement from them.

We discussed this patch internally and concluded the correct approach 
would be to not have the ethtool stats and use the VF info. Jakub beat 
us to the comment. We would plan to port the iproute implementation to 
OOT as well.

> Ack, but we really want people to move towards using standard stats.
> 
> Can you change the user space to first try reading the stats via
> iproute2/rtnetlink? And fallback to random ethtool strings if not
> available? That way it will work with any driver implementing the
> standard API. Long term that'll make everyone's life easier.
> 
> Out-of-tree code cannot be an argument upstream, otherwise we'd
> completely lose control over our APIs. Vendors could ship whatever
> in their out of tree repo and then force us to accept it upstream.
> 
> It's disappointing to see the vendor letting the uAPI of the out of
> tree driver diverge from upstream, especially a driver this mature.