netdev - Solving address deletion bottleneck in SCTP

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

Message-ID: <20241028124845.3866469-1-gnaaman@drivenets.com>
Date: Mon, 28 Oct 2024 12:48:43 +0000
From: Gilad Naaman <gnaaman@...venets.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
	Xin Long <lucien.xin@...il.com>,
	linux-sctp@...r.kernel.org,
	netdev@...r.kernel.org
Cc: Gilad Naaman <gnaaman@...venets.com>
Subject: Solving address deletion bottleneck in SCTP

Hello,

We've noticed that when a namespace has a large amount of IP addresses,
the list `net->sctp.local_addr_list` gets obscenely long.

This list contains both IPv4 and IPv6 addresses, of all scopes, and it is
a single long list, instead of a hashtable.

In our case we had 12K interfaces, each with an IPv4 and 2 IPv6 addresses
(GUA+LLA), which made deletion of a single address pretty expensive, since
it requires a linear search through 36K addresses.

Internally we solved it pretty naively by turning the list into hashmap, which
helped us avoid this bottleneck:

    + #define SCTP_ADDR_HSIZE_SHIFT	8
    + #define SCTP_ADDR_HSIZE		(1 << SCTP_ADDR_HSIZE_SHIFT)

    - 	struct list_head local_addr_list;
    + 	struct list_head local_addr_list[SCTP_ADDR_HSIZE];

I've used the same factor used by the IPv6 & IPv4 address tables.

I am not entirely sure this patch solves a big enough problem for the greater
general kernel community to warrant the increased memory usage (~2KiB-p-netns),
so I'll avoid sending it.

Recently, though, both IPv4 and IPv6 tables were namespacified, which makes
me think that maybe local_addr_list is no longer necessary, enabling us to
them directly instead of maintaining a separate list.

As far as I could tell, the only field of `struct sctp_sockaddr_entry` that
are used for items of this list, aside from the address itself, is the `valid`
bit, which can probably be folded into `struct in_ifaddr` and `struct inet6_ifaddr`.

What I'm suggesting, in short is:
 - Represent `valid` inside the original address structs.
 - Replace iteration of `local_addr_list` with iteration of ns addr tables
 - Eliminate `local_addr_list`

Is this a reasonable proposal?

Thank you for your time,
Gilad