pppoe-status fails when using ppp option ifname

[ixs]

The pppoe-status script will fail when pppd is using the option ifname xxx to set the ppp devicename to something other than pppX.

The logic in https://github.com/dfskoll/rp-pppoe/blob/master/scripts/pppoe-status#L66-L81 assumes, that the ppp-device.pid file contains just the process id of the pppd process.

When the parameter linkname is used however, the pid file contains two lines. The first line is the PID, the second line is the interface name. The corresponding pppd code is at https://github.com/ppp-project/ppp/blob/c2881a6b71a36d28a89166e82820dc5e711fd775/pppd/main.c#L826-L844 with the code adding the second line being at https://github.com/ppp-project/ppp/blob/c2881a6b71a36d28a89166e82820dc5e711fd775/pppd/main.c#L837-L838.

A quick fix that adds support for the two line pid file and allows pppoe-status to work again could look like this:

for i in /etc/ppp/ppp*.pid /var/run/ppp*.pid ; do
    if [ -r $i ] ; then
        if [ "$(wc -l $i | cut -c 1)" = 2 ]; then
            PID=`cat $i | head -n 1`
        else
            PID=`cat $i`
        fi
        if [ "$PID" = "$PPPD_PID" ] ; then
            if [ "$(wc -l $i | cut -c 1)" = 2 ]; then
                IF=`cat $i | tail -n 1`
            else
                IF=`basename $i .pid`
            fi
            $IP route | grep "dev ${IF}\s" > /dev/null
            if [ "$?" != "0" ] ; then
                echo "pppoe-status: Link is attached to $IF, but $IF is down"
                exit 1
            fi
            echo "pppoe-status: Link is up and running on interface $IF"
            $IP addr show $IF
            exit 0
        fi
    fi
done

But I am sure there's a nicer way...

[dfskoll]

Hi. Thanks for reporting this. I've committed https://github.com/dfskoll/rp-pppoe/commit/b826441a5a17597196148fba068da843263fa433 ; can you verify that it fixes the issue for you?

Regards,

Dianne.

[ixs]

The new code works for my usecase as the following excerpt from sh -x pppoe-status shows:

+ for i in '/etc/ppp/ppp*.pid' '/var/run/ppp*.pid'
+ '[' -r /var/run/ppp-tdsl0.pid ']'
++ head -n 1 /var/run/ppp-tdsl0.pid
+ PID=12306
++ tail -n +2 /var/run/ppp-tdsl0.pid
++ head -n 1
+ IF=tdsl0
+ test IF = ''
+ '[' 12306 = 12306 ']'
+ /sbin/ip route
+ grep 'dev tdsl0'
+ '[' 0 '!=' 0 ']'
+ echo 'pppoe-status: Link is up and running on interface tdsl0'

I am not 100% sure though about the logic of setting $IF to ppp0 in case there's no ifname in the pidfile: https://github.com/dfskoll/rp-pppoe/blob/master/scripts/pppoe-status#L63-L65 I fear that it works fine for a single ppp0 connection but will probably look at the wrong link in case there are multiple concurrent pppoe sessions if the ifname parameter is not used.

[ixs]

Had another look, this doesn't look like a complete fix yet.

During the pppoe connection phase, the IF parameter ends up being an empty string which breaks the successive grep over the route table resulting in a success message from the status script even though the connection isn't up yet.

++ head -n 1 /var/run/pppoe-adsl.pid.pppd
+ PPPD_PID=13253
++ tail -n +2 /var/run/pppoe-adsl.pid.pppd
++ head -n 1
+ IF=
+ test '' = ''
+ IF=ppp0
+ for i in '/etc/ppp/ppp*.pid' '/var/run/ppp*.pid'
+ '[' -r '/etc/ppp/ppp*.pid' ']'
+ for i in '/etc/ppp/ppp*.pid' '/var/run/ppp*.pid'
+ '[' -r /var/run/pppoe-adsl.pid ']'
++ head -n 1 /var/run/pppoe-adsl.pid
+ PID=13080
++ tail -n +2 /var/run/pppoe-adsl.pid
++ head -n 1
+ IF=
+ test IF = ''
+ '[' 13080 = 13253 ']'
+ for i in '/etc/ppp/ppp*.pid' '/var/run/ppp*.pid'
+ '[' -r /var/run/ppp-tdsl0.pid ']'
++ head -n 1 /var/run/ppp-tdsl0.pid
+ PID=13253
++ tail -n +2 /var/run/ppp-tdsl0.pid
++ head -n 1
+ IF=
+ test IF = ''
+ '[' 13253 = 13253 ']'
+ /sbin/ip route
+ grep 'dev '
+ '[' 0 '!=' 0 ']'
+ echo 'pppoe-status: Link is up and running on interface '
pppoe-status: Link is up and running on interface
+ /sbin/ip link show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: wan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
    link/ether 00:1d:aa:ec:9e:87 brd ff:ff:ff:ff:ff:ff
3: lan1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
    link/ether 0c:c4:7a:6c:e7:d4 brd ff:ff:ff:ff:ff:ff
4: lan2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
    link/ether 0c:c4:7a:6c:e7:d5 brd ff:ff:ff:ff:ff:ff
5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:bf:ce:00:5f:e8 brd ff:ff:ff:ff:ff:ff
6: lan3: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 0c:c4:7a:6c:e7:d6 brd ff:ff:ff:ff:ff:ff
7: lan4: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state DOWN mode DEFAULT group default qlen 1000
    link/ether 0c:c4:7a:6c:e7:d7 brd ff:ff:ff:ff:ff:ff
8: lan1.10@lan1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 0c:c4:7a:6c:e7:d4 brd ff:ff:ff:ff:ff:ff
10: ovpn_udp0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 100
    link/none
11: ovpn_tcp0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1300 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 100
    link/none
+ exit 0

[dfskoll]

D'oh, that was a typo. The line if test "IF" = "" ; then should obviously be if test "$IF" = "" ; then

[dfskoll]

I am not 100% sure though about the logic of setting $IF to ppp0 in case there's no ifname in the pidfile:

I agree that this doesn't make much sense, but I don't know of any other way to get the interface name. Unless pppd writes it into the pidfile somewhere, how else can we get it?

[ixs]

D'oh, that was a typo. The line if test "IF" = "" ; then should obviously be if test "$IF" = "" ; then

Much better with that fixed. 👍🏻

[ixs]

I am not 100% sure though about the logic of setting $IF to ppp0 in case there's no ifname in the pidfile:

I agree that this doesn't make much sense, but I don't know of any other way to get the interface name. Unless pppd writes it into the pidfile somewhere, how else can we get it?

You're right. There is no real way of getting the relation of which pppd is responsible for which ppp network interface short of parsing the logs if the pidfile doesn't have it. There's the basename $pidfile .pid trick but even that doesn't always work as pppd now seems to create two .pid files, one being ppp-<ifname>.pid and the other being <ifname>.pid. The new code is at https://github.com/ppp-project/ppp/blob/c2881a6b71a36d28a89166e82820dc5e711fd775/pppd/main.c#L833.

So maybe just assuming ppp0 when everything else fails, is the right approach... 🤔