diff --git a/core/commands/ping.go b/core/commands/ping.go index 6383af77666..23ca3f6a844 100644 --- a/core/commands/ping.go +++ b/core/commands/ping.go @@ -8,13 +8,13 @@ import ( "strings" "time" - "github.com/ipfs/go-ipfs/core" "github.com/ipfs/go-ipfs/core/commands/cmdenv" cmds "gx/ipfs/QmSXUokcP4TJpFfqozT69AVAYRtzXVMUjzQVkYX41R9Svs/go-ipfs-cmds" ma "gx/ipfs/QmT4U94DnD8FRfqr21obWY32HLM5VExccPKMjQHofeYqr9/go-multiaddr" "gx/ipfs/QmTRhk7cgjUf2gfQ3p2M9KPECNZEW9XUrmHcFCgog4cPgB/go-libp2p-peer" pstore "gx/ipfs/QmTTJcDL3gsnGDALjh2fDGg1onGRUdVgNL2hU2WEZcVrMX/go-libp2p-peerstore" + iaddr "gx/ipfs/QmZc5PLgxW61uTPG24TroxHDF6xzgbhZZQf5i53ciQC47Y/go-ipfs-addr" cmdkit "gx/ipfs/Qmde5VP1qUkyQXKCfmEUA7bP64V2HAptbJ7phuPp7jXWwg/go-ipfs-cmdkit" ) @@ -59,17 +59,17 @@ trip latency information. return ErrNotOnline } - addr, peerID, err := ParsePeerParam(req.Arguments[0]) + addr, pid, err := ParsePeerParam(req.Arguments[0]) if err != nil { return fmt.Errorf("failed to parse peer address '%s': %s", req.Arguments[0], err) } - if peerID == n.Identity { + if pid == n.Identity { return ErrPingSelf } if addr != nil { - n.Peerstore.AddAddr(peerID, addr, pstore.TempAddrTTL) // temporary + n.Peerstore.AddAddr(pid, addr, pstore.TempAddrTTL) // temporary } numPings, _ := req.Options[pingCountOptionName].(int) @@ -77,148 +77,96 @@ trip latency information. return fmt.Errorf("error: ping count must be greater than 0, was %d", numPings) } - outChan := pingPeer(req.Context, n, peerID, numPings) - - return res.Emit(outChan) - }, - Type: PingResult{}, - Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, out *PingResult) error { - if len(out.Text) > 0 { - fmt.Fprintln(w, out.Text) - } else if out.Success { - fmt.Fprintf(w, "Pong received: time=%.2f ms\n", out.Time.Seconds()*1000) - } else { - fmt.Fprintf(w, "Pong failed\n") - } - return nil - }), - }, -} - -func pingPeer(ctx context.Context, n *core.IpfsNode, pid peer.ID, numPings int) <-chan interface{} { - outChan := make(chan interface{}) - go func() { - defer close(outChan) - if len(n.Peerstore.Addrs(pid)) == 0 { // Make sure we can find the node in question - select { - case outChan <- &PingResult{ + if err := res.Emit(&PingResult{ Text: fmt.Sprintf("Looking up peer %s", pid.Pretty()), Success: true, - }: - case <-ctx.Done(): - return + }); err != nil { + return err } - ctx, cancel := context.WithTimeout(ctx, kPingTimeout) - defer cancel() + ctx, cancel := context.WithTimeout(req.Context, kPingTimeout) p, err := n.Routing.FindPeer(ctx, pid) + cancel() if err != nil { - select { - case outChan <- &PingResult{Text: fmt.Sprintf("Peer lookup error: %s", err)}: - case <-ctx.Done(): - return - } - - return + return res.Emit(&PingResult{Text: fmt.Sprintf("Peer lookup error: %s", err)}) } n.Peerstore.AddAddrs(p.ID, p.Addrs, pstore.TempAddrTTL) } - select { - case outChan <- &PingResult{ + if err := res.Emit(&PingResult{ Text: fmt.Sprintf("PING %s.", pid.Pretty()), Success: true, - }: - case <-ctx.Done(): - return + }); err != nil { + return err } - ctx, cancel := context.WithTimeout(ctx, kPingTimeout*time.Duration(numPings)) + ctx, cancel := context.WithTimeout(req.Context, kPingTimeout*time.Duration(numPings)) defer cancel() pings, err := n.Ping.Ping(ctx, pid) if err != nil { - select { - case outChan <- &PingResult{ + return res.Emit(&PingResult{ Success: false, Text: fmt.Sprintf("Ping error: %s", err), - }: - case <-ctx.Done(): - } - return + }) } - var done bool var total time.Duration - for i := 0; i < numPings && !done; i++ { + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + for i := 0; i < numPings; i++ { + t, ok := <-pings + if !ok { + break + } + + if err := res.Emit(&PingResult{ + Success: true, + Time: t, + }); err != nil { + return err + } + + total += t + select { + case <-ticker.C: case <-ctx.Done(): - done = true - break - case t, ok := <-pings: - if !ok { - done = true - break - } - select { - case outChan <- &PingResult{ - Success: true, - Time: t, - }: - case <-ctx.Done(): - return - } - total += t - time.Sleep(time.Second) + return ctx.Err() } } averagems := total.Seconds() * 1000 / float64(numPings) - select { - case outChan <- &PingResult{ + return res.Emit(&PingResult{ Success: true, Text: fmt.Sprintf("Average latency: %.2fms", averagems), - }: - case <-ctx.Done(): - } - }() - return outChan + }) + }, + Type: PingResult{}, + Encoders: cmds.EncoderMap{ + cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, out *PingResult) error { + if len(out.Text) > 0 { + fmt.Fprintln(w, out.Text) + } else if out.Success { + fmt.Fprintf(w, "Pong received: time=%.2f ms\n", out.Time.Seconds()*1000) + } else { + fmt.Fprintf(w, "Pong failed\n") + } + return nil + }), + }, } func ParsePeerParam(text string) (ma.Multiaddr, peer.ID, error) { - // to be replaced with just multiaddr parsing, once ptp is a multiaddr protocol - idx := strings.LastIndex(text, "/") - if idx == -1 { - pid, err := peer.IDB58Decode(text) - if err != nil { - return nil, "", err - } - - return nil, pid, nil - } - - addrS := text[:idx] - peeridS := text[idx+1:] - - var maddr ma.Multiaddr - var pid peer.ID - - // make sure addrS parses as a multiaddr. - if len(addrS) > 0 { - var err error - maddr, err = ma.NewMultiaddr(addrS) + // Multiaddr + if strings.HasPrefix(text, "/") { + a, err := iaddr.ParseString(text) if err != nil { return nil, "", err } + return a.Transport(), a.ID(), nil } - - // make sure idS parses as a peer.ID - var err error - pid, err = peer.IDB58Decode(peeridS) - if err != nil { - return nil, "", err - } - - return maddr, pid, nil + // Raw peer ID + p, err := peer.IDB58Decode(text) + return nil, p, err } diff --git a/test/sharness/t0041-ping.sh b/test/sharness/t0041-ping.sh index 1207dde5b00..bf5cf2ca33a 100755 --- a/test/sharness/t0041-ping.sh +++ b/test/sharness/t0041-ping.sh @@ -6,6 +6,8 @@ test_description="Test ping command" test_init_ipfs +BAD_PEER="QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJx" + # start iptb + wait for peering test_expect_success 'init iptb' ' iptb init -n 2 --bootstrap=none --port=0 @@ -23,6 +25,13 @@ test_expect_success "test ping other" ' ipfsi 1 ping -n2 -- "$PEERID_0" ' +test_expect_success "test ping unreachable peer" ' + printf "Looking up peer %s\n" "$BAD_PEER" > bad_ping_exp && + printf "Peer lookup error: routing: not found\n" >> bad_ping_exp && + ipfsi 0 ping -n2 -- "$BAD_PEER" > bad_ping_actual && + test_cmp bad_ping_exp bad_ping_actual +' + test_expect_success "test ping self" ' ! ipfsi 0 ping -n2 -- "$PEERID_0" && ! ipfsi 1 ping -n2 -- "$PEERID_1"