Skip to content
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9eea7cf
move to bind_to / bind_to_async / ditch bind_to_unspecified/ async
puhtaytow Jul 14, 2025
a26d7d5
swap remaining tests
puhtaytow Jul 14, 2025
d498b42
inline bind_to in meta
puhtaytow Jul 14, 2025
a45ca39
support range and single ports scenarios
puhtaytow Jul 17, 2025
66e0081
fix typo
puhtaytow Jul 17, 2025
073dee6
move tests to new bind_to api
puhtaytow Jul 17, 2025
a6dd863
fix import
puhtaytow Jul 17, 2025
c50db86
move to new unique api
puhtaytow Jul 17, 2025
c396d24
finish it
puhtaytow Jul 17, 2025
f62e018
cosmetics / bring docs part / change function names to more explicit
puhtaytow Jul 17, 2025
975fa39
remove obsolete function
puhtaytow Jul 17, 2025
ccb3dac
simplify the code
puhtaytow Jul 17, 2025
2d489d6
further reduce code
puhtaytow Jul 17, 2025
5140795
more explicit function naming
puhtaytow Jul 17, 2025
5123fea
reduce even more code
puhtaytow Jul 17, 2025
993e52c
adjust func name
puhtaytow Jul 17, 2025
98b8402
fix docs
puhtaytow Jul 17, 2025
9f0b88a
remove remaining bind_to
puhtaytow Jul 17, 2025
ee6aef8
bring removed import
puhtaytow Jul 17, 2025
ff43cf2
Merge pull request #2 from puhtaytow/swap-bind_to_unspecified-to-bind…
puhtaytow Jul 17, 2025
807dce4
allocate single port instead of many for cases that don't need more t…
puhtaytow Jul 17, 2025
4d2894b
move to range / adjust code
puhtaytow Jul 18, 2025
b5e1bc0
cleanup
puhtaytow Jul 18, 2025
f836247
cleanup round 2
puhtaytow Jul 18, 2025
738c73a
cleanup round 3
puhtaytow Jul 18, 2025
47c0048
add deprecation for bind_to_unspecified
puhtaytow Jul 18, 2025
796b912
remove the deprecation of bind_to_unspecified / too many things to fi…
puhtaytow Jul 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions core/src/repair/ancestor_hashes_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ mod test {
blockstore::make_many_slot_entries, get_tmp_ledger_path,
get_tmp_ledger_path_auto_delete, shred::Nonce,
},
solana_net_utils::bind_to_unspecified,
solana_net_utils::sockets::bind_to_localhost_unique,
solana_perf::packet::Packet,
solana_runtime::bank_forks::BankForks,
solana_signer::Signer,
Expand Down Expand Up @@ -1357,7 +1357,8 @@ mod test {
impl ManageAncestorHashesState {
fn new(bank_forks: Arc<RwLock<BankForks>>) -> Self {
let ancestor_hashes_request_statuses = Arc::new(DashMap::new());
let ancestor_hashes_request_socket = Arc::new(bind_to_unspecified().unwrap());
let ancestor_hashes_request_socket =
Arc::new(bind_to_localhost_unique().expect("should bind"));
let epoch_schedule = bank_forks
.read()
.unwrap()
Expand Down
18 changes: 10 additions & 8 deletions core/src/repair/repair_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1276,9 +1276,8 @@ mod test {
get_tmp_ledger_path_auto_delete,
shred::max_ticks_per_n_shreds,
},
solana_net_utils::{
bind_to_unspecified,
sockets::{bind_to, localhost_port_range_for_tests},
solana_net_utils::sockets::{
bind_to, bind_to_localhost_unique, localhost_port_range_for_tests,
},
solana_runtime::bank::Bank,
solana_signer::Signer,
Expand Down Expand Up @@ -1662,7 +1661,7 @@ mod test {
};
let mut duplicate_slot_repair_statuses = HashMap::new();
let dead_slot = 9;
let receive_socket = &bind_to_unspecified().unwrap();
let receive_socket = &bind_to_localhost_unique().expect("should bind");
let duplicate_status = DuplicateSlotRepairStatus {
correct_ancestor_to_repair: (dead_slot, Hash::default()),
start_ts: u64::MAX,
Expand Down Expand Up @@ -1691,7 +1690,7 @@ mod test {
&blockstore,
&serve_repair,
&mut RepairStats::default(),
&bind_to_unspecified().unwrap(),
&bind_to_localhost_unique().expect("should bind"),
&None,
&RwLock::new(OutstandingRequests::default()),
&identity_keypair,
Expand All @@ -1717,7 +1716,7 @@ mod test {
&blockstore,
&serve_repair,
&mut RepairStats::default(),
&bind_to_unspecified().unwrap(),
&bind_to_localhost_unique().expect("should bind"),
&None,
&RwLock::new(OutstandingRequests::default()),
&identity_keypair,
Expand All @@ -1736,7 +1735,7 @@ mod test {
&blockstore,
&serve_repair,
&mut RepairStats::default(),
&bind_to_unspecified().unwrap(),
&bind_to_localhost_unique().expect("should bind"),
&None,
&RwLock::new(OutstandingRequests::default()),
&identity_keypair,
Expand All @@ -1751,7 +1750,10 @@ mod test {
let bank_forks = BankForks::new_rw_arc(bank);
let dummy_addr = Some((
Pubkey::default(),
bind_to_unspecified().unwrap().local_addr().unwrap(),
bind_to_localhost_unique()
.expect("should bind")
.local_addr()
.unwrap(),
));
let cluster_info = Arc::new(new_test_cluster_info());
let ledger_path = get_tmp_ledger_path_auto_delete!();
Expand Down
43 changes: 33 additions & 10 deletions net-utils/src/sockets.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
#[cfg(feature = "dev-context-only-utils")]
use tokio::net::UdpSocket as TokioUdpSocket;
use {
crate::PortRange,
log::warn,
socket2::{Domain, SockAddr, Socket, Type},
std::{
io,
net::{IpAddr, SocketAddr, TcpListener, UdpSocket},
net::{IpAddr, Ipv4Addr, SocketAddr, TcpListener, UdpSocket},
sync::atomic::{AtomicU16, Ordering},
},
};
#[cfg(feature = "dev-context-only-utils")]
use {std::net::Ipv4Addr, tokio::net::UdpSocket as TokioUdpSocket};
// base port for deconflicted allocations
const BASE_PORT: u16 = 5000;
// how much to allocate per individual process.
// we expect to have at most 64 concurrent tests in CI at any moment on a given host.
const SLICE_PER_PROCESS: u16 = (u16::MAX - BASE_PORT) / 64;
/// Retrieve a free 20-port slice for unit tests
///
/// When running under nextest, this will try to provide
/// a unique slice of port numbers (assuming no other nextest processes
/// are running on the same host) based on NEXTEST_TEST_GLOBAL_SLOT variable
Expand All @@ -25,9 +23,9 @@ const SLICE_PER_PROCESS: u16 = (u16::MAX - BASE_PORT) / 64;
/// When running without nextest, this will only bump an atomic and eventually
/// panic when it runs out of port numbers to assign.
#[allow(clippy::arithmetic_side_effects)]
pub fn localhost_port_range_for_tests() -> (u16, u16) {
pub fn unique_port_range_for_tests(size: u16) -> (u16, u16) {
static SLICE: AtomicU16 = AtomicU16::new(0);
let offset = SLICE.fetch_add(20, Ordering::Relaxed);
let offset = SLICE.fetch_add(size, Ordering::Relaxed);
let start = offset
+ match std::env::var("NEXTEST_TEST_GLOBAL_SLOT") {
Ok(slot) => {
Expand All @@ -40,8 +38,29 @@ pub fn localhost_port_range_for_tests() -> (u16, u16) {
}
Err(_) => BASE_PORT,
};
assert!(start < u16::MAX - 20, "ran out of port numbers!");
(start, start + 20)
assert!(start < u16::MAX - size, "ran out of port numbers!");
(start, start + size)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have an off by one error here? If start is 10000, and i want 20 ports, i get back (10000, 10020) which is 21 ports.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good question, i wasn't confident enough so decided to check it with the test and the code below pass positively.

        let (pr_s, pr_e) = localhost_port_range_for_tests();
        assert_eq!(pr_e - pr_s, 20);
        ...

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well ya the difference between 10020 and 10000 is 20. but if we return both of them, the user will use them all. but that is 21 ports returned even though the difference is 20.
in your example above, if i use every port from pr_e to pr_s, that is 21 ports. if we count it out: 0, 1, 2, 3, ..., 19, 20. that is 21.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, would you suggest to add note in the function docs or maybe return Range? instead?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switching to range would be great, yes. Since this is a new function we might as well have proper API for it.

}

/// Retrieve a free 20-port slice for unit tests
///
/// When running under nextest, this will try to provide
/// a unique slice of port numbers (assuming no other nextest processes
/// are running on the same host) based on NEXTEST_TEST_GLOBAL_SLOT variable
/// The port ranges will be reused following nextest logic.
///
/// When running without nextest, this will only bump an atomic and eventually
/// panic when it runs out of port numbers to assign.
pub fn localhost_port_range_for_tests() -> (u16, u16) {
unique_port_range_for_tests(20)
}

/// Bind a `UdpSocket` to a unique port.
pub fn bind_to_localhost_unique() -> io::Result<UdpSocket> {
bind_to(
IpAddr::V4(Ipv4Addr::LOCALHOST),
localhost_port_range_for_tests().0,
)
Comment thread
puhtaytow marked this conversation as resolved.
}

pub fn bind_gossip_port_in_range(
Expand Down Expand Up @@ -246,7 +265,11 @@ pub async fn bind_to_async(ip_addr: IpAddr, port: u16) -> io::Result<TokioUdpSoc

#[cfg(feature = "dev-context-only-utils")]
pub async fn bind_to_localhost_async() -> io::Result<TokioUdpSocket> {
bind_to_async(IpAddr::V4(Ipv4Addr::LOCALHOST), 0).await
bind_to_async(
IpAddr::V4(Ipv4Addr::LOCALHOST),
localhost_port_range_for_tests().0,
)
.await
}

#[cfg(feature = "dev-context-only-utils")]
Expand Down
4 changes: 2 additions & 2 deletions rpc-test/tests/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
solana_commitment_config::CommitmentConfig,
solana_hash::Hash,
solana_keypair::Keypair,
solana_net_utils::bind_to_unspecified,
solana_net_utils::sockets::bind_to_localhost_unique,
solana_pubkey::Pubkey,
solana_pubsub_client::nonblocking::pubsub_client::PubsubClient,
solana_rent::Rent,
Expand Down Expand Up @@ -290,7 +290,7 @@ fn test_rpc_subscriptions() {
let test_validator =
TestValidator::with_no_fees_udp(alice.pubkey(), None, SocketAddrSpace::Unspecified);

let transactions_socket = bind_to_unspecified().unwrap();
let transactions_socket = bind_to_localhost_unique().expect("should bind");
transactions_socket.connect(test_validator.tpu()).unwrap();

let rpc_client = RpcClient::new(test_validator.rpc_url());
Expand Down
6 changes: 3 additions & 3 deletions streamer/src/nonblocking/sendmmsg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ mod tests {
sendmmsg::SendPktsError,
},
assert_matches::assert_matches,
solana_net_utils::sockets::{bind_to_localhost_async, bind_to_unspecified_async},
solana_net_utils::sockets::bind_to_localhost_async,
solana_packet::PACKET_DATA_SIZE,
std::{
io::ErrorKind,
Expand Down Expand Up @@ -177,7 +177,7 @@ mod tests {
];
let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4];

let sender = bind_to_unspecified_async().await.expect("bind");
let sender = bind_to_localhost_async().await.expect("should bind");
let res = batch_send(&sender, &packet_refs[..]).await;
assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
let res = multi_target_send(&sender, &packets[0], &dest_refs).await;
Expand All @@ -189,7 +189,7 @@ mod tests {
let packets: Vec<_> = (0..5).map(|_| vec![0u8; PACKET_DATA_SIZE]).collect();
let ipv4local = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 8080);
let ipv4broadcast = SocketAddr::new(IpAddr::V4(Ipv4Addr::BROADCAST), 8080);
let sender = bind_to_unspecified_async().await.expect("bind");
let sender = bind_to_localhost_async().await.expect("should bind");

// test intermediate failures for batch_send
let packet_refs: Vec<_> = vec![
Expand Down
6 changes: 3 additions & 3 deletions streamer/src/sendmmsg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ mod tests {
sendmmsg::{batch_send, multi_target_send, SendPktsError},
},
assert_matches::assert_matches,
solana_net_utils::{bind_to_localhost, bind_to_unspecified},
solana_net_utils::{bind_to_localhost, sockets::bind_to_localhost_unique},
solana_packet::PACKET_DATA_SIZE,
std::{
io::ErrorKind,
Expand Down Expand Up @@ -362,7 +362,7 @@ mod tests {
];
let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4];

let sender = bind_to_unspecified().expect("bind");
let sender = bind_to_localhost_unique().expect("should bind");
let res = batch_send(&sender, packet_refs);
assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
let res = multi_target_send(&sender, &packets[0], &dest_refs);
Expand All @@ -374,7 +374,7 @@ mod tests {
let packets: Vec<_> = (0..5).map(|_| vec![0u8; PACKET_DATA_SIZE]).collect();
let ipv4local = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 8080);
let ipv4broadcast = SocketAddr::new(IpAddr::V4(Ipv4Addr::BROADCAST), 8080);
let sender = bind_to_unspecified().expect("bind");
let sender = bind_to_localhost_unique().expect("should bind");

// test intermediate failures for batch_send
let packet_refs: Vec<_> = vec![
Expand Down
4 changes: 2 additions & 2 deletions turbine/src/broadcast_stage/standard_broadcast_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ mod test {
get_tmp_ledger_path,
shred::{max_ticks_per_n_shreds, DATA_SHREDS_PER_FEC_BLOCK},
},
solana_net_utils::bind_to_unspecified,
solana_net_utils::sockets::bind_to_localhost_unique,
solana_runtime::bank::Bank,
solana_signer::Signer,
solana_streamer::socket::SocketAddrSpace,
Expand Down Expand Up @@ -531,7 +531,7 @@ mod test {
leader_keypair.clone(),
SocketAddrSpace::Unspecified,
));
let socket = bind_to_unspecified().unwrap();
let socket = bind_to_localhost_unique().expect("should bind");
let mut genesis_config = create_genesis_config(10_000).genesis_config;
genesis_config.ticks_per_slot = max_ticks_per_n_shreds(num_shreds_per_slot, None) + 1;

Expand Down
4 changes: 2 additions & 2 deletions validator/src/admin_rpc_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ mod tests {
create_genesis_config, create_genesis_config_with_leader, GenesisConfigInfo,
},
},
solana_net_utils::bind_to_unspecified,
solana_net_utils::sockets::bind_to_localhost_unique,
solana_program_option::COption,
solana_program_pack::Pack,
solana_pubkey::Pubkey,
Expand Down Expand Up @@ -1030,7 +1030,7 @@ mod tests {
vote_account,
repair_whitelist,
notifies: Arc::new(RwLock::new(KeyUpdaters::default())),
repair_socket: Arc::new(bind_to_unspecified().unwrap()),
repair_socket: Arc::new(bind_to_localhost_unique().expect("should bind")),
outstanding_repair_requests: Arc::<
RwLock<repair_service::OutstandingShredRepairs>,
>::default(),
Expand Down
Loading