On Mon, Nov 4, 2024 at 3:36 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>
Acked-by: Xin Long <lucien.xin@xxxxxxxxx>
Thanks.
> ---
> Changes in v2:
> - Reorder list removal to avoid race with new sessions
> ---
> net/sctp/ipv6.c | 2 +-
> net/sctp/protocol.c | 16 +++++++++++++++-
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index f7b809c0d142..b96c849545ae 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -103,10 +103,10 @@ static int sctp_inet6addr_event(struct notifier_block *this, unsigned long ev,
> ipv6_addr_equal(&addr->a.v6.sin6_addr,
> &ifa->addr) &&
> addr->a.v6.sin6_scope_id == ifa->idev->dev->ifindex) {
> - sctp_addr_wq_mgmt(net, addr, SCTP_ADDR_DEL);
> found = 1;
> addr->valid = 0;
> list_del_rcu(&addr->list);
> + sctp_addr_wq_mgmt(net, addr, SCTP_ADDR_DEL);
> break;
> }
> }
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 39ca5403d4d7..8b9a1b96695e 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;
> + }
> +
> /* Offsets existing events in addr_wq */
> addrw = sctp_addr_wq_lookup(net, addr);
> if (addrw) {
> @@ -808,10 +822,10 @@ static int sctp_inetaddr_event(struct notifier_block *this, unsigned long ev,
> if (addr->a.sa.sa_family == AF_INET &&
> addr->a.v4.sin_addr.s_addr ==
> ifa->ifa_local) {
> - sctp_addr_wq_mgmt(net, addr, SCTP_ADDR_DEL);
> found = 1;
> addr->valid = 0;
> list_del_rcu(&addr->list);
> + sctp_addr_wq_mgmt(net, addr, SCTP_ADDR_DEL);
> break;
> }
> }
> --
> 2.34.1
>
