Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added wrapper type and todos #348

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

added wrapper type and todos #348

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c05499
added wrapper type and todos
AnkurRathore Nov 4, 2024
ea1b51f
modified code to support ipv4 or ipv6 connection
AnkurRathore Dec 26, 2024
9e6e101
added function for iplookup and made enums in separate file
AnkurRathore Dec 27, 2024
f802b0f
added the required enum and made the required changes
AnkurRathore Dec 28, 2024
edd1b3a
added functionality to respect ip version preferences
AnkurRathore Jan 15, 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
103 changes: 48 additions & 55 deletions rama-tcp/src/client/connect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ impl<C>ConnectIpMode<C>{
}
}

/*
TODO - Using the wrapper types
in the tcp_connect fn, update the dns type to use the DnsResolveIpMode
*/


/// Trait used internally by [`tcp_connect`] and the `TcpConnector`
/// to actually establish the [`TcpStream`.]
Expand Down Expand Up @@ -197,8 +194,8 @@ async fn tcp_connect_inner<State, Dns, Connector>(
ctx: &Context<State>,
domain: Domain,
port: u16,
dns: Dns,
connector: Connector,
dns: DnsResolveIpMode<Dns>,
connector: ConnectIpMode<Connector>,
) -> Result<(TcpStream, SocketAddr), OpaqueError>
where
State: Clone + Send + Sync + 'static,
Expand All @@ -210,37 +207,41 @@ where
let connected = Arc::new(AtomicBool::new(false));
let sem = Arc::new(Semaphore::new(3));

// IPv6
let ipv6_tx = tx.clone();
let ipv6_domain = domain.clone();
let ipv6_connected = connected.clone();
let ipv6_sem = sem.clone();
ctx.spawn(tcp_connect_inner_branch(
dns.clone(),
connector.clone(),
IpKind::Ipv6,
ipv6_domain,
port,
ipv6_tx,
ipv6_connected,
ipv6_sem,
));

// IPv4
let ipv4_tx = tx;
let ipv4_domain = domain.clone();
let ipv4_connected = connected.clone();
let ipv4_sem = sem;
ctx.spawn(tcp_connect_inner_branch(
dns,
connector,
IpKind::Ipv4,
ipv4_domain,
port,
ipv4_tx,
ipv4_connected,
ipv4_sem,
));
match dns.mode {
DnsResolveIpMode::SingleIpV4 | DnsResolveIpMode::DualPreferIpV4 | DnsResovleIpMode::Dual =>{
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to provide utility methods to DnsResolveIpMode, such as:

fn ipv4_supported(&self) -> bool;
fn ipv6_supported(&self) -> bool;

Is same-same, but easier to have to update such statements only in a single loc.

//IPV4
ctx.spawn(tcp_connect_inner_branch(
dns.clone(),
Copy link
Member

Choose a reason for hiding this comment

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

You can add Copy derive trait to these mode Enums, then you don't need to Clone

connector.clone(),
IpKind::Ipv4,
domain.clone(),
port,
tx.clone(),
connected.clone(),
sem.clone(),
));
}
_ => {}

}

match dns.mode {
DnsResolveIpMode::SingleIpV6 | DnsResovleIpMode::Dual =>{
//IPV6
ctx.spawn(tcp_connect_inner_branch(
dns.clone(),
connector.clone(),
IpKind::Ipv6,
domain.clone(),
port,
tx.clone(),
connected.clone(),
sem.clone(),
));
}
_ => {}

}

// wait for the first connection to succeed,
// ignore the rest of the connections (sorry, but not sorry)
Expand Down Expand Up @@ -274,39 +275,31 @@ async fn tcp_connect_inner_branch<Dns, Connector>(
Dns: DnsResolver<Error: Into<BoxError>> + Clone,
Connector: TcpStreamConnector<Error: Into<BoxError> + Send + 'static> + Clone,
{
/*TODO
modify the match to use dns.ip_mode and match for
SingleIpv4 or DualPreferIpv4 or Dual
*/

let ip_it = match ip_kind {
IpKind::Ipv4 => match dns.ipv4_lookup(domain).await {
IpKind::Ipv4 if matches!(dns.mode, DnsResolveIpMode::Dual |DnsResolveIpMode::SingleIpV4 |DnsResolveIpMode::DualPreferIpV4 ) =>{
Copy link
Member

@GlenDC GlenDC Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would suggest to do:

match ip_kind => {
     IpKind::Ipv4 => if dns.mode.ipv4_supported() {
           // ... logic
     } else {
          trace::debug ipv4 not supported by DnsMode: no connection can be established
     }

     IpKind::Ipv6 => if dns.mode.ipv6_supported() {
           // ... logic
     } else {
          trace::debug ipv6 not supported by DnsMode: no connection can be established
     }

This way we have a clear trace in our logs / spans

match dns.ipv4_lookup(domain).await {
Ok(ips) => Either::A(ips.into_iter().map(IpAddr::V4)),
Err(err) => {
let err = OpaqueError::from_boxed(err.into());
tracing::trace!(err = %err, "[{ip_kind:?}] failed to resolve domain to IPv4 addresses");
return;
}
},
/*TODO
modify the match to use dns.ip_mode and match for
SingleIpv6 or Dual
*/
IpKind::Ipv6 => match dns.ipv6_lookup(domain).await {
}
},
IpKind::Ipv6 if matches!(dns.mode, DnsResolveIpMode::Dual |DnsResolveIpMode::SingleIpV6)=> {
match dns.ipv6_lookup(domain).await {
Ok(ips) => Either::B(ips.into_iter().map(IpAddr::V6)),
Err(err) => {
let err = OpaqueError::from_boxed(err.into());
tracing::trace!(err = ?err, "[{ip_kind:?}] failed to resolve domain to IPv6 addresses");
return;
}
},
}
},
_ => return,
};

/*
TODO
Not sure here, we need to get the matched ip type and
use it
*/

for (index, ip) in ip_it.enumerate() {
let addr = (ip, port).into();

Expand Down