[Bps-public-commit] djabberd branch, srv-lookup, created. 6620836b19e2101f0c08ec75ff8df1a3b0e7ccfd

Alex Vandiver alexmv at bestpractical.com
Wed May 26 01:25:39 EDT 2010


The branch, srv-lookup has been created
        at  6620836b19e2101f0c08ec75ff8df1a3b0e7ccfd (commit)

- Log -----------------------------------------------------------------
commit 044a7b580cce65a9b8116737b4bbaff2f455460b
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Mar 8 22:23:52 2010 -0500

    Refactor resolver to eliminate duplicate code, allow for non A or SRV requests

diff --git a/lib/DJabberd/DNS.pm b/lib/DJabberd/DNS.pm
index 9434601..fd1ec11 100644
--- a/lib/DJabberd/DNS.pm
+++ b/lib/DJabberd/DNS.pm
@@ -6,6 +6,7 @@ use fields (
             'callback',
             'srv',
             'port',
+            'type',
             'recurse_count',
             'became_readable',  # bool
             'timed_out',        # bool
@@ -45,32 +46,12 @@ sub srv {
         }
     }
 
-    my $pkt = Net::DNS::Packet->new("$service.$hostname", "SRV", "IN");
-
-    $logger->debug("pkt = $pkt");
-    my $sock = $resolver->bgsend($pkt);
-    $logger->debug("sock = $sock");
-    my $self = $class->SUPER::new($sock);
-
-    $self->{hostname} = $hostname;
-    $self->{callback} = $callback;
-    $self->{srv}      = $service;
-    $self->{port}     = $port;
-    $self->{recurse_count} = $recurse_count;
-
-    $self->{became_readable} = 0;
-    $self->{timed_out}       = 0;
-
-    # TODO: make DNS timeout configurable
-    Danga::Socket->AddTimer(5.0, sub {
-        return if $self->{became_readable};
-        $self->{timed_out} = 1;
-        $logger->debug("DNS 'SRV' lookup for '$hostname' timed out");
-        $callback->();
-        $self->close;
-    });
-
-    $self->watch_read(1);
+    $class->resolve(
+        type     => 'SRV',
+        domain   => "$service.$hostname",
+        callback => $callback,
+        port     => $port,
+    );
 }
 
 sub new {
@@ -93,23 +74,52 @@ sub new {
         return;
     }
 
+    $class->resolve(
+        type     => 'A',
+        domain   => $hostname,
+        callback => $callback,
+        port     => $port,
+    );
+}
 
-    my $sock = $resolver->bgsend($hostname);
+sub resolve {
+    my ($class, %opts) = @_;
+
+    foreach (qw(callback domain type port)) {
+        croak("No '$_' field") unless $opts{$_};
+    }
+
+    my $hostname = delete $opts{'domain'};
+    my $callback = delete $opts{'callback'};
+    my $type     = delete $opts{'type'};
+    my $port     = delete $opts{'port'};
+    my $recurse_count = delete($opts{'recurse_count'}) || 0;
+    croak "unknown opts" if %opts;
+
+    my $method = "event_read_" . lc $type;
+    croak "unknown type $type" unless $class->can($method);
+
+    my $pkt = Net::DNS::Packet->new($hostname, $type, "IN");
+
+    $logger->debug("pkt = $pkt");
+    my $sock = $resolver->bgsend($pkt);
+    $logger->debug("sock = $sock");
     my $self = $class->SUPER::new($sock);
 
     $self->{hostname} = $hostname;
     $self->{callback} = $callback;
     $self->{port}     = $port;
+    $self->{type}     = $type;
     $self->{recurse_count} = $recurse_count;
 
     $self->{became_readable} = 0;
     $self->{timed_out}       = 0;
 
-    # TODO: make DNS timeout configurable, remove duplicate code
+    # TODO: make DNS timeout configurable
     Danga::Socket->AddTimer(5.0, sub {
         return if $self->{became_readable};
         $self->{timed_out} = 1;
-        $logger->debug("DNS 'A' lookup for '$hostname' timed out");
+        $logger->debug("DNS '$type' lookup for '$hostname' timed out");
         $callback->();
         $self->close;
     });
@@ -128,24 +138,22 @@ sub event_read {
     }
     $self->{became_readable} = 1;
 
-    if ($self->{srv}) {
-        $logger->debug("DNS socket $self->{sock} became readable for 'srv'");
-        return $self->event_read_srv;
-    } else {
-        $logger->debug("DNS socket $self->{sock} became readable for 'a'");
-        return $self->event_read_a;
-    }
+    $logger->debug("DNS socket $self->{sock} became readable for '$self->{type}'");
+
+    my $method = "event_read_" . lc $self->{type};
+    return $self->$method;
 }
 
-sub event_read_a {
+sub read_packets {
     my $self = shift;
+    return $resolver->bgread($self->{sock})->answer;
+}
 
-    my $sock = $self->{sock};
-    my $cb   = $self->{callback};
-
-    my $packet = $resolver->bgread($sock);
+sub event_read_a {
+    my $self = shift;
 
-    my @ans = $packet->answer;
+    my $cb = $self->{callback};
+    my @ans = $self->read_packets;
 
     for my $ans (@ans) {
         my $rv = eval {
@@ -187,11 +195,8 @@ sub event_read_a {
 sub event_read_srv {
     my $self = shift;
 
-    my $sock = $self->{sock};
-    my $cb   = $self->{callback};
-
-    my $packet = $resolver->bgread($sock);
-    my @ans = $packet->answer;
+    my $cb = $self->{callback};
+    my @ans = $self->read_packets;
 
     # FIXME: is this right?  right order and direction?
     my @targets = sort {
@@ -202,7 +207,7 @@ sub event_read_srv {
     unless (@targets) {
         # no result, fallback to an A lookup
         $self->close;
-        $logger->debug("DNS socket $sock for 'srv' had nothing, falling back to 'a' lookup");
+        $logger->debug("DNS socket for 'srv' had nothing, falling back to 'a' lookup");
         DJabberd::DNS->new(hostname => $self->{hostname},
                            port     => $self->{port},
                            callback => $cb);
@@ -210,7 +215,7 @@ sub event_read_srv {
     }
 
     # FIXME:  we only do the first target now.  should do a chain.
-    $logger->debug("DNS socket $sock for 'srv' found stuff, now doing hostname lookup on " . $targets[0]->target);
+    $logger->debug("DNS socket for 'srv' found stuff, now doing hostname lookup on " . $targets[0]->target);
     DJabberd::DNS->new(hostname => $targets[0]->target,
                        port     => $targets[0]->port,
                        callback => $cb);

commit 1428e9bbff35da4c7a89c64ead9ff8ac90c287c2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Mar 8 23:28:27 2010 -0500

    Move "A" record fallback to calling function, where we have the right hostname value

diff --git a/lib/DJabberd/DNS.pm b/lib/DJabberd/DNS.pm
index fd1ec11..44af40c 100644
--- a/lib/DJabberd/DNS.pm
+++ b/lib/DJabberd/DNS.pm
@@ -46,10 +46,21 @@ sub srv {
         }
     }
 
+    my $try_a = sub {
+        my @values = @_;
+        return $callback->(@values) if @values;
+        $logger->debug("DNS socket for 'srv' had nothing, falling back to 'a' lookup");
+        DJabberd::DNS->new(
+            hostname => $hostname,
+            port     => $port,
+            callback => $callback,
+        );
+    };
+
     $class->resolve(
         type     => 'SRV',
         domain   => "$service.$hostname",
-        callback => $callback,
+        callback => $try_a,
         port     => $port,
     );
 }
@@ -204,15 +215,9 @@ sub event_read_srv {
         $a->weight   <=> $b->weight
     } grep { ref $_ eq "Net::DNS::RR::SRV" && $_->port } @ans;
 
-    unless (@targets) {
-        # no result, fallback to an A lookup
-        $self->close;
-        $logger->debug("DNS socket for 'srv' had nothing, falling back to 'a' lookup");
-        DJabberd::DNS->new(hostname => $self->{hostname},
-                           port     => $self->{port},
-                           callback => $cb);
-        return;
-    }
+    $self->close;
+
+    return $cb->() unless @targets;
 
     # FIXME:  we only do the first target now.  should do a chain.
     $logger->debug("DNS socket for 'srv' found stuff, now doing hostname lookup on " . $targets[0]->target);

commit cc6607a2e0517ed4bb87fc1d25954232a995a5d2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Mar 8 23:29:53 2010 -0500

    For debugging, include the domain name which gave the IP in question during DNS lookups

diff --git a/lib/DJabberd/DNS.pm b/lib/DJabberd/DNS.pm
index 44af40c..281499a 100644
--- a/lib/DJabberd/DNS.pm
+++ b/lib/DJabberd/DNS.pm
@@ -186,7 +186,7 @@ sub event_read_a {
                 $logger->debug("Ignoring RR response for $self->{hostname}");
             }
             else {
-                $cb->(DJabberd::IPEndPoint->new($ans->address, $self->{port}));
+                $cb->(DJabberd::IPEndPoint->new($ans->address, $self->{port}, $self->{hostname}));
             }
             $self->close;
             1;
@@ -229,10 +229,15 @@ sub event_read_srv {
 
 package DJabberd::IPEndPoint;
 sub new {
-    my ($class, $addr, $port) = @_;
-    return bless { addr => $addr, port => $port };
+    my ($class, $addr, $port, $name) = @_;
+    if (defined $name) {
+        $name = lc $name;
+        $name =~ s/\.$//;
+    }
+    return bless { addr => $addr, port => $port, name => $name };
 }
 
+sub name { $_[0]{name} }
 sub addr { $_[0]{addr} }
 sub port { $_[0]{port} }
 

commit 6620836b19e2101f0c08ec75ff8df1a3b0e7ccfd
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Mar 8 23:34:36 2010 -0500

    Fix SRV weighting code, and note what the actual FIXME is

diff --git a/lib/DJabberd/DNS.pm b/lib/DJabberd/DNS.pm
index 281499a..5a7e4a1 100644
--- a/lib/DJabberd/DNS.pm
+++ b/lib/DJabberd/DNS.pm
@@ -209,10 +209,12 @@ sub event_read_srv {
     my $cb = $self->{callback};
     my @ans = $self->read_packets;
 
-    # FIXME: is this right?  right order and direction?
+    # FIXME: Should nominally do weighted random choice beteen records
+    # with lowest priority, not just choose the highest weighted.  See
+    # RFC 2782.
     my @targets = sort {
         $a->priority <=> $b->priority ||
-        $a->weight   <=> $b->weight
+        $b->weight   <=> $a->weight
     } grep { ref $_ eq "Net::DNS::RR::SRV" && $_->port } @ans;
 
     $self->close;

-----------------------------------------------------------------------



More information about the Bps-public-commit mailing list