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.

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

add Ping tool to api 07/19/2022 #440

Merged
merged 3 commits into from
Jul 20, 2022
Merged

add Ping tool to api 07/19/2022 #440

merged 3 commits into from
Jul 20, 2022

Conversation

aanon4
Copy link
Contributor

@aanon4 aanon4 commented Jul 19, 2022

This is a simple ping tool to complement the existing iperf3 tool. It allows someone to initiate a ping test from any node to any other node. The test is limited to 10 seconds.

The goal is to enable improved network monitoring by allowing specific links in the network to be tested. Right now, although nodes can be tested, the path will always be from the initiator to the target, which is not necessarily what you want.

The use mirrors the "api" we use for the iperf tool.

@aanon4 aanon4 requested review from dman776 and ae6xe July 19, 2022 23:21
Copy link
Contributor

@ab7pa ab7pa left a comment

Choose a reason for hiding this comment

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

Works well on my test node.

Screen Shot 2022-07-19 at 4 32 57 PM

@dman776
Copy link
Contributor

dman776 commented Jul 20, 2022

What do you think about adding a getPing function to aredn.utils and exposing in the api. This is the same approach as the traceroute takes. http://localnode.local.mesh:8080/cgi-bin/api?traceroute=TARGETNODENAME
it could be: http://192.168.0.47:8080/cgi-bin/api?ping=TARGETNODENAME

Copy link
Contributor

@dman776 dman776 left a comment

Choose a reason for hiding this comment

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

see comment about adding to api

@aanon4
Copy link
Contributor Author

aanon4 commented Jul 20, 2022

I had no idea that was even there!
Could do - was just using iperf as a basis for this, but could use traceroute instead - not fussed either way

@ab7pa
Copy link
Contributor

ab7pa commented Jul 20, 2022

I'll wait to wrap up the documentation for this feature until we decide whether to accomplish it through cgi-bin/ping or cgi-bin/api -- either way it looks like a good addition. I suppose that cgi-bin/iperf could eventually be converted to the api approach at some point. Thanks, @aanon4 Tim!

@dman776
Copy link
Contributor

dman776 commented Jul 20, 2022

May as well move them to api for future. We can always have a "html" readable page in the interim that essentially calls the api and formats for html if you think it's necessary. That is my strong preference.

@dman776
Copy link
Contributor

dman776 commented Jul 20, 2022 via email

@aanon4 aanon4 requested a review from dman776 July 20, 2022 01:22
@aanon4
Copy link
Contributor Author

aanon4 commented Jul 20, 2022

@dman776 Tried to match the traceroute api with what seems like sensible json data. Output looks like this:

{
   "pages": {
     "ping": {
       "kn6plv-services": {
         "summary": {
           "tx": "11",
           "minMs": "0.517",
           "rx": "10",
           "lossPercentage": "9",
           "maxMs": "0.698",
           "avgMs": "0.573",
           "ip": "10.72.238.17"
         },
         "pings": [
           {
             "timeMs": "0.669",
             "ttl": "64",
             "seq": "0",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.596",
             "ttl": "64",
             "seq": "1",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.569",
             "ttl": "64",
             "seq": "2",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.540",
             "ttl": "64",
             "seq": "3",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.532",
             "ttl": "64",
             "seq": "4",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.517",
             "ttl": "64",
             "seq": "5",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.517",
             "ttl": "64",
             "seq": "6",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.698",
             "ttl": "64",
             "seq": "7",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.572",
             "ttl": "64",
             "seq": "8",
             "ip": "10.72.238.17"
           },
           {
             "timeMs": "0.529",
             "ttl": "64",
             "seq": "9",
             "ip": "10.72.238.17"
           }
         ]
       }
     }
   },
   "api_version": "1.4"
 }

Copy link
Contributor

@dman776 dman776 left a comment

Choose a reason for hiding this comment

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

need to increment the api version to 1.5

@dman776
Copy link
Contributor

dman776 commented Jul 20, 2022

also, is the ttl value needed? It's always going to be the same 64, right?

@aanon4
Copy link
Contributor Author

aanon4 commented Jul 20, 2022

Bump.
TTL - I decided to include all the information I was given, because maybe it would be useful.

Copy link
Contributor

@dman776 dman776 left a comment

Choose a reason for hiding this comment

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

looks good.

@dman776 dman776 merged commit d166393 into aredn:main Jul 20, 2022
@dman776 dman776 changed the title Ping tool (to complement the iperf3 tool) add Ping tool to api 07/19/2022 Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants