[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aff1d8c7-08b7-46dd-ba86-43f23f70a668@redhat.com>
Date: Thu, 11 Apr 2019 15:01:19 +0800
From: Jason Wang <jasowang@...hat.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>,
David Woodhouse <dwmw2@...radead.org>, netdev@...r.kernel.org
Subject: Re: Tun congestion/BQL
On 2019/4/10 下午9:42, Toke Høiland-Jørgensen wrote:
> Jason Wang <jasowang@...hat.com> writes:
>
>> On 2019/4/10 下午9:01, David Woodhouse wrote:
>>> On Wed, 2019-04-10 at 15:01 +0300, David Woodhouse wrote:
>>>> --- a/drivers/net/tun.c
>>>> +++ b/drivers/net/tun.c
>>>> @@ -1125,7 +1128,9 @@ static netdev_tx_t tun_net_xmit(struct sk_buff
>>>> *skb, struct net_device *dev)
>>>> if (tfile->flags & TUN_FASYNC)
>>>> kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
>>>> tfile->socket.sk->sk_data_ready(tfile->socket.sk);
>>>>
>>>> + if (!ptr_ring_empty(&tfile->tx_ring))
>>>> + netif_stop_queue(tun->dev);
>>>> rcu_read_unlock();
>>>> return NETDEV_TX_OK;
>>>>
>>>>
>>> Hm, that should be using ptr_ring_full() shouldn't it? So...
>>>
>>> --- a/drivers/net/tun.c
>>> +++ b/drivers/net/tun.c
>>> @@ -1121,6 +1121,9 @@ static netdev_tx_t tun_net_xmit(struct s
>>> if (ptr_ring_produce(&tfile->tx_ring, skb))
>>> goto drop;
>>>
>>> + if (ptr_ring_full(&tfile->tx_ring))
>>> + netif_stop_queue(tun->dev);
>>> +
>>> /* Notify and wake up reader process */
>>> if (tfile->flags & TUN_FASYNC)
>>> kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
>>> @@ -2229,6 +2232,7 @@ static ssize_t tun_do_read(struct tun_st
>>> consume_skb(skb);
>>> }
>>>
>>> + netif_wake_queue(tun->dev);
>>> return ret;
>>> }
>>>
>>>
>>> That doesn't seem to make much difference at all; it's still dropping a
>>> lot of packets because ptr_ring_produce() is returning non-zero.
>>
>> I think you need try to stop the queue just in this case? Ideally we may
>> want to stop the queue when the queue is about to full, but we don't
>> have such helper currently.
> Ideally we want to react when the queue starts building rather than when
> it starts getting full; by pushing back on upper layers (or, if
> forwarding, dropping packets to signal congestion).
>
> In practice, this means tuning the TX ring to the *minimum* size it can
> be without starving (this is basically what BQL does for Ethernet), and
> keeping packets queued in the qdisc layer instead, where it can be
> managed...
>
> -Toke
Yes, but we do have user that don't use qdisc at all.
Thanks
Powered by blists - more mailing lists