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 1 commit into
base: main
Choose a base branch
from
+57 −0
Open

added wrapper type and todos #348

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 57 additions & 0 deletions rama-tcp/src/client/connect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,49 @@ use tokio::{
},
};

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum IPModes {
Copy link
Member

Choose a reason for hiding this comment

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

Originally I wanted to go for a single IP Mode indeed, and work with wrapper types. However I think it makes more sense to define 2 enums:

enum DnsResolveIpMode
    dual (default)
    singleIpV4
    singleIpV6
    dualPreferIpv4

enum ConnectIpMode
    dual (default)
    ipv4
    ipv6

Don't think these belong in rama-tcp though. Probably more like in rama-net or so.

Copy link
Author

Choose a reason for hiding this comment

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

Do I need to put these in rama-net?

Copy link
Member

Choose a reason for hiding this comment

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

Yes as these are not specific to tcp. E.g. in a future version we'll also support udp. And other packages such as Dns might also make use of it for w/e. The common denominator is that it's all related to network protocols, as such rama-net.

You can put it in a file created as rama-net/src/mode.rs and just pub mod mode in the lib.rs file of that crate.

Dual,
SingleIpV4,
SingleIpV6,
DualPreferIpV4
}

impl Default for IPModes {
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 easier define this by using Default in the #[derive(..)] above and set #[default] above Dual,.

fn default() -> Self {
Self::Dual
}
}

//DNS Resolver
GlenDC marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Clone)]
struct DnsResolveIpMode<D>{
resolver: D,
mode: IPModes
}

impl<D> DnsResolveIpMode<D>{
fn new(resolver:D, mode: IPModes) -> Self {
Self { resolver, mode}
}
}

struct ConnectIpMode<C>{
connector: C,
ip_mode: IPModes
}

impl<C>ConnectIpMode<C>{
fn new(connector: C, ip_mode: IPModes) -> Self {
Self {connector, ip_mode}
}
}

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

Choose a reason for hiding this comment

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

I think it's in async fn tcp_connect_inner<State, Dns, Connector>( that you'll want to check for DnsResolveIpMode.

And it's in the location where we actually establish a tcp stream that you'll check for ConnectIpMode.

Originally I was thinking to also automatically let one influence the other, but I think it makes more sense to just let the user worry about that. E.g. if user selects Ipv6-only for DNS and ipv4-only for Connect, then that obviously will fail, but that is up to the user to then fix. No point in trying to do anything about that, as the only reasonable thing to do would be to return some kind of error, so let's not bother with that as the current error will already be fine enough.

*/

/// Trait used internally by [`tcp_connect`] and the `TcpConnector`
/// to actually establish the [`TcpStream`.]
pub trait TcpStreamConnector: Send + Sync + 'static {
Expand Down Expand Up @@ -231,6 +274,10 @@ 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 {
Ok(ips) => Either::A(ips.into_iter().map(IpAddr::V4)),
Expand All @@ -240,6 +287,10 @@ async fn tcp_connect_inner_branch<Dns, Connector>(
return;
}
},
/*TODO
modify the match to use dns.ip_mode and match for
SingleIpv6 or Dual
*/
IpKind::Ipv6 => match dns.ipv6_lookup(domain).await {
Ok(ips) => Either::B(ips.into_iter().map(IpAddr::V6)),
Err(err) => {
Expand All @@ -250,6 +301,12 @@ async fn tcp_connect_inner_branch<Dns, Connector>(
},
};

/*
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