Re: [PATCH net-next 2/3] virtio-net: transmit napi

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Sun, Apr 2, 2017 at 10:30 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Apr 02, 2017 at 04:10:11PM -0400, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@google.com>
>>
>> Convert virtio-net to a standard napi tx completion path. This enables
>> better TCP pacing using TCP small queues and increases single stream
>> throughput.
>>
>> The virtio-net driver currently cleans tx descriptors on transmission
>> of new packets in ndo_start_xmit. Latency depends on new traffic, so
>> is unbounded. To avoid deadlock when a socket reaches its snd limit,
>> packets are orphaned on tranmission. This breaks socket backpressure,
>> including TSQ.
>>
>> Napi increases the number of interrupts generated compared to the
>> current model, which keeps interrupts disabled as long as the ring
>> has enough free descriptors. Keep tx napi optional for now. Follow-on
>> patches will reduce the interrupt cost.
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 63 ++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 6aac0ad0d9b2..95d938e82080 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -33,9 +33,10 @@
>>  static int napi_weight = NAPI_POLL_WEIGHT;
>>  module_param(napi_weight, int, 0444);
>>
>> -static bool csum = true, gso = true;
>> +static bool csum = true, gso = true, napi_tx = true;
>>  module_param(csum, bool, 0444);
>>  module_param(gso, bool, 0444);
>> +module_param(napi_tx, bool, 0644);
>>
>>  /* FIXME: MTU in config. */
>>  #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
>
> Off by default seems safer until we can find better ways
> to reduce the overhead, esp for UDP.

Okay, I'll change that. I don't have an immediate idea for that
unidirectional case.