G_GET_NET fails if default route not static

owendelong commented 1 year ago

Creating a bug report/issue

Required Information

DietPi version | G_DIETPI_VERSION_CORE=8 G_DIETPI_VERSION_SUB=20 G_DIETPI_VERSION_RC=1 G_GITBRANCH='master' G_GITOWNER='MichaIng' G_LIVE_PATCH_STATUS[0]='applied'
Distro version | bullseye (NanoPi R6S)
Kernel version | Linux <host> 5.10.160 #1 SMP Tue Jul 4 16:02:09 CST 2023 aarch64 GNU/Linux
SBC model | NanoPi 6 series (aarch64)
Power supply used | PoE Adapter (20W capable)
SD card used eMMC onboard

Additional Information (if applicable)

Using FRR for routing, taking full tables via BGP and getting default via OSPF
Fresh
Yes

Steps to reproduce

Install DietPi
Install FRR (for OSPF)
Set up an OSPF neighbor that will advertise a default route to the DietPi host
Configure OPSF routing on the Dietpi and validate neighborship and receipt of default route

The G_GET_NET() function in /boot/dietpi/func/dietpi-globals expects routing table output in the form of a static route entry only: e.g. 192.124.40.112/30 dev eth0 proto kernel scope link src 192.124.40.113

When Default route is learned via OSPF, the routing entry returned by "ip r 0/0" is slightly different: e.g. default nhid 15 via 192.124.40.114 dev eth0 proto ospf metric 20

G_GET_NET does not adapt and results in most dietpi related software that prints ip information returning "Device 192.124.40.114 does not exist" as it attempts to retrieve interface configuration. Because the field count in the mawk command is off by 2, the next-hop IP address ($5) is treated as the interface instead of the actual interface ($7).

Expected behaviour

Function G_GET_NET should adapt to all expected forms of the output of the "ip route list" command. Also, scripts should (as general good form) use the full text versions of commands (e.g. "ip route list" instead of "ip r l").

Actual behaviour

As described above, the incorrect field from "ip route list" command is being treated as the device.

Extra details

/boot/dietpi/func/dietpi-globals G_GET_NET starts at line 1252 Errant line of code is at line 1293 (Note, I did not test with IPv6 yet, so 1294 may also have a similar problem).

I found that the following replacement of line 1293 works for my system:

                        [[ $fam != '-6' ]] && (
                          local droute f2
                          droute=`ip route list 0/0`
                          f2=`echo $droute | mawk '{ print $2;exit }'`
                          if [ "$f2" = 'nhid' ]; then
                            # Dynamic route format
                            read -r gateway if < <(echo $droute ${iface:+dev "$iface"} | mawk '{ print $5, $7;exit}')
                          else
                            # Assume static format
                            read -r gateway if < <(echo $droute ${iface:+dev "$iface"} | mawk '{ print $3, $5;exit}')
                          fi
                        )

YMMV, and I'm not a BASH expert, so there may be better ways to write it.

MichaIng commented 1 year ago

Many thanks for your suggestion.

Best is probably to do everything in awk: Loop through input fields, store those after a field matched "via" and "dev", and print those two:

ip r l 0/0 | mawk '{for(i=0;i<NF;i++) if($i=="via") {g=$(i+1)} else if($i=="dev") {d=$(i+1)}; print g,d;exit}'

So we have a generic method which does not only cover your case but also other possible default route entries, and it does not reduce performance by doing multiple command calls based on logic outside of awk.

Function G_GET_NET should adapt to all expected forms of the output of the "ip route list" command. Also, scripts should (as general good form) use the full text versions of commands (e.g. "ip route list" instead of "ip r l").

You are right that full commands are common practice in coding. I however never adapted to this but instead like to keep lines short and save the few bytes by used the max shortcuts. If a contributor/reviewed really does not know the shortcuts, it is possibly valuable to learn about them and speed up managing such stuff on console a little 😉. So let's keep focusing on the functional enhancement and in case discuss coding standards in a separate issue.

MichaIng commented 1 year ago

Added this to the dev branch now and releasing the beta today: https://github.com/MichaIng/DietPi/commit/bff37d1

owendelong commented 1 year ago

Agree that your suggestion is a better way. This was what I hacked together quickly while trying to solve other problems to work around the distraction. Further discussion of coding standards can be left for a discussion over an adult beverage if we ever meet in person.

Appreciate all you have done to make DietPi as useful as it is.

MichaIng / DietPi