netdev - Re: [PATCH net-next 1/3] virtio-net: fix possible dim status unrecoverable

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]

Message-ID: <c3d23a1d-eb26-4da8-91c6-99097e4cfc31@linux.alibaba.com>
Date: Wed, 17 Jan 2024 11:34:35 +0800
From: Heng Qi <hengqi@...ux.alibaba.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org, virtualization@...ts.linux.dev,
 Jason Wang <jasowang@...hat.com>, Paolo Abeni <pabeni@...hat.com>,
 Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
 "David S. Miller" <davem@...emloft.net>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Subject: Re: [PATCH net-next 1/3] virtio-net: fix possible dim status
 unrecoverable

在 2024/1/16 下午9:15, Michael S. Tsirkin 写道:
> On Tue, Jan 16, 2024 at 09:11:31PM +0800, Heng Qi wrote:
>> When the dim worker is scheduled, if it fails to acquire the lock,
>> dim may not be able to return to the working state later.
>>
>> For example, the following single queue scenario:
>>    1. The dim worker of rxq0 is scheduled, and the dim status is
>>       changed to DIM_APPLY_NEW_PROFILE;
>>    2. The ethtool command is holding rtnl lock;
>>    3. Since the rtnl lock is already held, virtnet_rx_dim_work fails
>>       to acquire the lock and exits;
>>
>> Then, even if net_dim is invoked again, it cannot work because the
>> state is not restored to DIM_START_MEASURE.
>>
>> Fixes: 6208799553a8 ("virtio-net: support rx netdim")
>> Signed-off-by: Heng Qi <hengqi@...ux.alibaba.com>
>> ---
>> Belongs to the net branch.
>>
>>   drivers/net/virtio_net.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index d7ce4a1..f6ac3e7 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -3524,8 +3524,10 @@ static void virtnet_rx_dim_work(struct work_struct *work)
>>   	struct dim_cq_moder update_moder;
>>   	int i, qnum, err;
>>   
>> -	if (!rtnl_trylock())
>> +	if (!rtnl_trylock()) {
>> +		schedule_work(&dim->work);
>>   		return;
>> +	}
>>   
>>   	/* Each rxq's work is queued by "net_dim()->schedule_work()"
>>   	 * in response to NAPI traffic changes. Note that dim->profile_ix
> OK but this means that in cleanup it is not sufficient to flush
> dim work - it can requeue itself.

We did not use the flush work operation, cancel_work_sync will handle 
the re-queue situation:

    "Cancel @work and wait for its execution to finish. This function
    can be used even if the work re-queues itself or migrates to
    another workqueue. On return from this function, @work is
    guaranteed to be not pending or executing on any CPU."

Thanks.

>
>
>> -- 
>> 1.8.3.1