linux-kernel - Re: [RFC v3] net/core: add optional threading for backlog processing

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>] [day] [month] [year] [list]

Message-ID: <632b578a-10c7-a478-6e89-e44815fcec66@nbd.name>
Date:   Mon, 20 Feb 2023 09:23:03 +0100
From:   Felix Fietkau <nbd@....name>
To:     Hillf Danton <hdanton@...a.com>
Cc:     netdev@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC v3] net/core: add optional threading for backlog processing

On 20.02.23 01:41, Hillf Danton wrote:
> On Sun, 19 Feb 2023 14:10:05 +0100 Felix Fietkau <nbd@....name>
>>  /* Network device is going away, flush any packets still pending */
>>  static void flush_backlog(struct work_struct *work)
>>  {
>> +	unsigned int process_queue_empty;
>> +	bool threaded, flush_processq;
>>  	struct sk_buff *skb, *tmp;
>>  	struct softnet_data *sd;
>>  
>> @@ -5792,8 +5794,15 @@ static void flush_backlog(struct work_struct *work)
>>  			input_queue_head_incr(sd);
>>  		}
>>  	}
>> +
>> +	threaded = test_bit(NAPI_STATE_THREADED, &sd->backlog.state);
>> +	flush_processq = threaded &&
>> +			 !skb_queue_empty_lockless(&sd->process_queue);
>>  	rps_unlock_irq_enable(sd);
>>  
>> +	if (threaded)
>> +		goto out;
>> +
>>  	skb_queue_walk_safe(&sd->process_queue, skb, tmp) {
>>  		if (skb->dev->reg_state == NETREG_UNREGISTERING) {
>>  			__skb_unlink(skb, &sd->process_queue);
>> @@ -5801,7 +5810,16 @@ static void flush_backlog(struct work_struct *work)
>>  			input_queue_head_incr(sd);
>>  		}
>>  	}
>> +
>> +out:
>>  	local_bh_enable();
>> +
>> +	while (flush_processq) {
>> +		msleep(1);
>> +		rps_lock_irq_disable(sd);
>> +		flush_processq = process_queue_empty == sd->process_queue_empty;
>> +		rps_unlock_irq_enable(sd);
>> +	}
>>  }
> 
> Have hard time guessing how this wait works given process_queue_empty
> not initialized. Is this random check intended?
Please check my initial reply to this patch. I forgot to amend the 
commit with the missing initialization lines before sending out the patch.

- Felix