Laurent Julliard
3/16/2008 8:12:00 AM
Joel VanderWerf wrote:
> Joel VanderWerf wrote:
>> In icmp.rb, the problem seems to be that it uses process ID as the
>> ICMP packet ID, so it is impossible to tell which ECHO REPLY packet
>> corresponds to which request (if multiple requests come from one
>> process).
>
> On second thought, the problem could be solved in icmp.rb by using the
> sequence number to separate requests, and keeping the sequence number in
> an ICMP class var instead of instance vars. Here's a patch that seems to
> make your original code work correctly (detecting some hosts up and some
> down).
>
Joel,
Fist of all thanks for catching this!
Your recommendation is to turn the sequence number into a class variable
so shouldn't the sequence number really be named @@seq rather than @seq
if we want the sequence number to be unique across multiple instances of
ICMP objects?
And if this is so, one of the consequence is that for a given ICMP
instance chances are that the seq field in the icmp packet will not
contain consecutive values. I don't know if this is a problem wrt to the
ICMP specifications or not.
Don't you think an alternative would be not to use the PID as the packet
identifier but rather a class variable of our own that we would
increment each time a new ICMP instance is created?
Thanks again for your comments.
Laurent
> ---
> /usr/local/lib/ruby/gems/1.8/gems/net-ping-1.2.2/lib/net/ping/icmp.rb
> 2008-01-23 19:46:14.000000000 -0800
> +++ net/ping/icmp.rb 2008-03-15 13:09:55.000000000 -0700
> @@ -24,7 +24,6 @@
> def initialize(host=nil, port=nil, timeout=5)
> raise 'requires root privileges' if Process.euid > 0
>
> - @seq = 0
> @bind_port = 0
> @bind_host = nil
> @data_size = 56
> @@ -37,6 +36,11 @@
> super(host, port, timeout)
> @port = nil # This value is not used in ICMP pings.
> end
> +
> + @seq = 0
> + def self.next_seq
> + @seq = (@seq + 1) % 65536
> + end
>
> # Sets the number of bytes sent in the ping method.
> #
> @@ -73,14 +77,14 @@
> socket.bind(saddr)
> end
>
> - @seq = (@seq + 1) % 65536
> + seq = ICMP.next_seq
> pstring = 'C2 n3 A' << @data_size.to_s
> timeout = @timeout
>
> checksum = 0
> - msg = [ICMP_ECHO, ICMP_SUBCODE, checksum, @pid, @seq,
> @data].pack(pstring)
> + msg = [ICMP_ECHO, ICMP_SUBCODE, checksum, @pid, seq,
> @data].pack(pstring)
> checksum = checksum(msg)
> - msg = [ICMP_ECHO, ICMP_SUBCODE, checksum, @pid, @seq,
> @data].pack(pstring)
> + msg = [ICMP_ECHO, ICMP_SUBCODE, checksum, @pid, seq,
> @data].pack(pstring)
>
> start_time = Time.now
>
> @@ -101,7 +105,7 @@
> end
>
> pid = nil
> - seq = nil
> + rcv_seq = nil
>
> data, sender = socket.recvfrom(1500)
> port, host = Socket.unpack_sockaddr_in(sender)
> @@ -110,15 +114,15 @@
> case type
> when ICMP_ECHOREPLY
> if data.length >= 28
> - pid, seq = data[24, 4].unpack('n3')
> + pid, rcv_seq = data[24, 4].unpack('n3')
> end
> else
> if data.length > 56
> - pid, seq = data[52, 4].unpack('n3')
> + pid, rcv_seq = data[52, 4].unpack('n3')
> end
> end
>
> - if pid == @pid && seq == @seq && type == ICMP_ECHOREPLY
> + if pid == @pid && rcv_seq == seq && type == ICMP_ECHOREPLY
> bool = true
> end
> }
>
>