Re: [PATCH net-next] virtio_net: ethtool tx napi configuration

[Florian Fainelli <f.fainelli@gmail.com>]

On 9/12/2018 11:07 AM, Willem de Bruijn wrote:
> On Wed, Sep 12, 2018 at 1:42 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 9/9/2018 3:44 PM, Willem de Bruijn wrote:
>>> From: Willem de Bruijn <willemb@google.com>
>>>
>>> Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
>>> Interrupt moderation is currently not supported, so these accept and
>>> display the default settings of 0 usec and 1 frame.
>>>
>>> Toggle tx napi through a bit in tx-frames. So as to not interfere
>>> with possible future interrupt moderation, use bit 10, well outside
>>> the reasonable range of real interrupt moderation values.
>>>
>>> Changes are not atomic. The tx IRQ, napi BH and transmit path must
>>> be quiesced when switching modes. Only allow changing this setting
>>> when the device is down.
>>
>> Humm, would not a private ethtool flag to switch TX NAPI on/off be more
>> appropriate rather than use the coalescing configuration API here?
> 
> What do you mean by private ethtool flag? A new field in ethtool
> --features (-k)?

I meant using ethtool_drvinfo::n_priv_flags, ETH_SS_PRIV_FLAGS and then 
ETHTOOL_GFPFLAGS and ETHTOOL_SPFLAGS to control the toggling of that 
private flag. mlx5 has a number of privates flags for instance.

> 
> Configurable napi-tx is not a common feature across devices. We really
> want virtio-net to also just convert to napi-tx as default, but need a
> way to gradually convert with application opt-out if some workloads
> see regressions.

The rationale makes sense, no questions about it.

> There is prior art in interpreting coalesce values as
> more than a direct mapping to usec. The e1000 is one example.
>

Looked at both e1000 and e1000e and they both have a similar programming 
of the HW's interrupt target rate register, which is relevant to 
interrupt coalescing, what part of these drivers do you see as doing 
something not quite coalescing related?
-- 
Florian