Re: [PATCH net-next] sctp: Avoid enqueuing addr events redundantly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 29 Oct 2024, at 18:02, Xin Long <lucien.xin@xxxxxxxxx> wrote:
> 
> CAUTION: External E-Mail - Use caution with links and attachments
> 
> 
> On Mon, Oct 28, 2024 at 4:10 AM Gilad Naaman <gnaaman@xxxxxxxxxxxxx> wrote:
>> 
>> Avoid modifying or enqueuing new events if it's possible to tell that no
>> one will consume them.
>> 
>> Since enqueueing requires searching the current queue for opposite
>> events for the same address, adding addresses en-masse turns this
>> inetaddr_event into a bottle-neck, as it will get slower and slower
>> with each address added.
>> 
>> Signed-off-by: Gilad Naaman <gnaaman@xxxxxxxxxxxxx>
>> ---
>> net/sctp/protocol.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>> 
>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>> index 39ca5403d4d7..2e548961b740 100644
>> --- a/net/sctp/protocol.c
>> +++ b/net/sctp/protocol.c
>> @@ -738,6 +738,20 @@ void sctp_addr_wq_mgmt(struct net *net, struct sctp_sockaddr_entry *addr, int cm
>>         */
>> 
>>        spin_lock_bh(&net->sctp.addr_wq_lock);
>> +
>> +       /* Avoid searching the queue or modifying it if there are no consumers,
>> +        * as it can lead to performance degradation if addresses are modified
>> +        * en-masse.
>> +        *
>> +        * If the queue already contains some events, update it anyway to avoid
>> +        * ugly races between new sessions and new address events.
>> +        */
>> +       if (list_empty(&net->sctp.auto_asconf_splist) &&
>> +           list_empty(&net->sctp.addr_waitq)) {
>> +               spin_unlock_bh(&net->sctp.addr_wq_lock);
>> +               return;
> 
> What if after this but before the addr is deleted from local_addr_list in
> sctp_inetaddr_event(), a new SCTP association is created with these addrs
> in local_addr_list, will it miss this asconf addr_del?
> 
> Thanks.

Great point.

I think this can be solved by making sure `list_del_rcu(&addr->list);` is called
before `sctp_addr_wq_mgmt`.

I’ll resend this after I manage to un-bork my git-send-email config.

Thank you!

> 
>> +       }
>> +
>>        /* Offsets existing events in addr_wq */
>>        addrw = sctp_addr_wq_lookup(net, addr);
>>        if (addrw) {
>> --
>> 2.46.0
>> 





[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     SCTP

  Powered by Linux