ocochard / BSDRP

BSD Router Project
https://bsdrp.net
Other
177 stars 38 forks source link

Patch Proposal - graphpath #20

Closed vermaden closed 6 years ago

vermaden commented 6 years ago

Hi,

thanks for great tool.

I tried to use it with 3G/tun0 interface and its not very well suited towards that.

I made some modifications and here is the patch of these mods.

% diff -u ./graphpath.sh.ORG ./graphpath.sh
--- ./graphpath.sh.ORG  2018-04-25 09:57:09.547820000 +0200
+++ ./graphpath.sh      2018-04-25 10:12:27.483938000 +0200
@@ -149,6 +149,10 @@
 EOF
        fi
        printf '| IP:   %-18s   |\n' ${source_gateway}
+else
+cat <<EOF
++----------------------------+
+EOF
 fi # [ "${source_gateway}" != "LAN" ]

 printf '| ARP:  %-18s   |\n' ${source_gateway_arp}
@@ -168,10 +172,13 @@
 +----------------------------+
 EOF
 printf '| IF:   %-18s   |\n' ${source_interface}
-printf '| MAC:  %-18s   |\n' ${source_interface_mac}
+if [ "x${source_interface_mac}" != "x" ]
+then
+  printf '| MAC:  %-18s   |\n' ${source_interface_mac}
+fi
 printf '| IP:   %-18s   |\n' ${source_interface_ip}
 printf '| net:  %-18s   |\n' ${source_destination}
-printf '| mask: %-18s   |\n' ${source_mask}
+# printf '| mask: %-18s   |\n' ${source_mask}
 cat <<EOF
 |                            |
 |         THIS ${device}        |
@@ -185,7 +192,10 @@
 fi
 if [ "${source_interface}" != ${destination_interface} ]; then
        printf '| IP:   %-18s   |\n' ${destination_interface_ip}
-       printf '| MAC:  %-18s   |\n' ${destination_interface_mac}
+       if [ "x${destination_interface_mac}" != "x" ]
+       then
+         printf '| MAC:  %-18s   |\n' ${destination_interface_mac}
+       fi
        printf '| IF:   %-18s   |\n' ${destination_interface}
 fi
 cat <<EOF

Here is the result from non modified graphpath script.

% ./graphpath.sh.ORG 37.249.221.39 1.1.1.1 
This tool is mainly designed for drawing router or firewall routing view
+----------------------------+
|         SOURCE HOST        |
| IP:   37.249.221.39        |
+----------------------------+
              |
| ARP:  empty                |
+----------------------------+
              |
+----------------------------+
| IF:   lo0                  |
| MAC:                       |
| IP:   127.0.0.1            |
| net:  37.249.221.39        |
./graphpath.sh.ORG: source_mask: parameter not set

Here is the fixed one with patch.

% ./graphpath.sh 37.249.221.39 1.1.1.1
This tool is mainly designed for drawing router or firewall routing view
+----------------------------+
|         SOURCE HOST        |
| IP:   37.249.221.39        |
+----------------------------+
              |
+----------------------------+
| ARP:  empty                |
+----------------------------+
              |
+----------------------------+
| IF:   lo0                  |
| IP:   127.0.0.1            |
| net:  37.249.221.39        |
|                            |
|         THIS HOST          |
|                            |
| net:  0.0.0.0              |
| mask: 0.0.0.0              |
| IP:   37.249.221.39        |
| IF:   tun0                 |
+----------------------------+

Regards.

ocochard commented 6 years ago

Thanks, I've rewrote the script, can you check if the latest version has the same problem ?

vermaden commented 6 years ago

Now its not better but different :)

Here is output without modifications.

% ./graphpath-new.sh.ORG 46.215.0.141 1.1.1.1 
This tool is mainly designed for drawing router or firewall routing view
./graphpath-new.sh.ORG: position: parameter not set

There seams to be problem with postition and position parameter name.

% grep -C 3 postition ./graphpath-new.sh.ORG
        local ip=$3          # IP address for src & dst host (=direct for a router)
        local gateway=$4     #  IP address for a router
        local gateway_arp=$5 # ARP cache if directly connected to "me" block
        local postition=$6
        left=""
        right=""
        # There is a special case: If ip=direct, this mean this block is useless
% grep -C 3 position ./graphpath-new.sh.ORG 
        # There is a special case: If ip=direct, this mean this block is useless
        [ "$ip" == direct ] && return 0
        # If in second family mode, need to move or add vertical line
        [ "${position}" == "left" ] && left="                  |               "
        [ "${position}" == "right" ] && right="                  |"
        # If label is "ROUTER TOWARDS DESTINATION", need to move this block to the left
        #echo ${label} | grep -q "ROUTER TOWARDS DESTINATION" && left="                  |               "
        # If label is "ROUTER TOWARDS SOURCE", need to add an vertical line to the left"

Here is the script after my 'new' modifications.


% ./graphpath-new.sh 46.215.0.141 1.1.1.1
This tool is mainly designed for drawing router or firewall routing view
+----------------------------+
| SOURCE HOST                |
| IP:   46.215.0.141         |
| ARP:  empty                |
+----------------------------+
              |               
+----------------------------+
| IF:   lo0                  |
| MAC:                       |
| IP:   127.0.0.1            |
| net:  46.215.0.141         |
| mask: 255.255.255.255      |
|                            |
|         THIS HOST          |
|                            |
| net:  0.0.0.0              |
| mask: 0.0.0.0              |
| IP:   46.215.0.141         |
| MAC:                       |
| IF:   tun0                 |
+----------------------------+
              |
+----------------------------+
| DESTINATION HOST           |
| IP:   1.1.1.1              |
| ARP:  empty                |
+----------------------------+

... and here is the 'new' diff.

% diff -u ./graphpath-new.sh.ORG ./graphpath-new.sh    
--- ./graphpath-new.sh.ORG      2018-04-27 12:28:59.563936000 +0200
+++ ./graphpath-new.sh  2018-04-27 12:33:02.593642000 +0200
@@ -132,14 +132,10 @@
        local ip=$3          # IP address for src & dst host (=direct for a router)
        local gateway=$4     #  IP address for a router
        local gateway_arp=$5 # ARP cache if directly connected to "me" block
-       local postition=$6
        left=""
        right=""
        # There is a special case: If ip=direct, this mean this block is useless
        [ "$ip" == direct ] && return 0
-       # If in second family mode, need to move or add vertical line
-       [ "${position}" == "left" ] && left="                  |               "
-       [ "${position}" == "right" ] && right="                  |"
        # If label is "ROUTER TOWARDS DESTINATION", need to move this block to the left
        #echo ${label} | grep -q "ROUTER TOWARDS DESTINATION" && left="                  |               "
        # If label is "ROUTER TOWARDS SOURCE", need to add an vertical line to the left"
ocochard commented 6 years ago

thanks, this typo is fixed, and the new output with this version seems better than the previous.

vermaden commented 6 years ago

Still a problem with 'position' variable, as $6 does not exist and its not check for not existence.

% ./graphpath-new-new.sh 46.215.0.141 1.1.1.1         
This tool is mainly designed for drawing router or firewall routing view
+----------------------------+
| SOURCE HOST                |
| IP:   46.215.0.141         |
| ARP:  empty                |
+----------------------------+
              |               
+----------------------------+
| IF:   lo0                  |
| MAC:                       |
| IP:   127.0.0.1            |
| net:  46.215.0.141         |
| mask: 255.255.255.255      |
|                            |
|         THIS HOST          |
|                            |
| net:  0.0.0.0              |
| mask: 0.0.0.0              |
| IP:   46.215.0.141         |
| MAC:                       |
| IF:   tun0                 |
+----------------------------+
./graphpath-new-new.sh: 6: parameter not set

% grep position ./graphpath-new-new.sh
        local position=$6
        [ "${position}" == "left" ] && left="                  |               "
        [ "${position}" == "right" ] && right="                  |"
ocochard commented 6 years ago

I've forgot to update arguments on one function call.

ocochard commented 6 years ago

Can I close this ticket ?

vermaden commented 6 years ago

If its fixed then sure, go ahead.