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

luci-proto-3g/ppp/pppossh: fix being unable to set keepalive to 0 #7392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Preport
Copy link

@Preport Preport commented Nov 14, 2024

Since on openwrt keepalive option defaults to "5 1" when it's not defined
(see https://github.com/openwrt/openwrt/blob/6720c4ccba256186bf2f1b1edadb851c447e62a5/package/network/services/ppp/files/ppp.sh#L128).
Users must be able to set it to 0 to ignore connection failures.

  • This PR is not from my main or master branch 💩, but a separate branch ✅
  • Each commit has a valid ✒️ Signed-off-by: <my@email.address> row (via git commit --signoff)
  • Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
  • Tested on: (mt7621 mipsel, openwrt 23.05.04 firefox) ✅
  • Mention: @jow-
  • Closes: LCP echo failure threshold 0 does not behave as described in LuCI #2112
  • Description: (describe the changes proposed in this PR)

Since on openwrt keepalive option defaults to "5 1" when it's not defined (https://github.com/openwrt/openwrt/blob/6720c4ccba256186bf2f1b1edadb851c447e62a5/package/network/services/ppp/files/ppp.sh#L128).
Users must be able to set it to 0 to ignore connection failures.

Signed-off-by: Erdem Gez <perport@perport.net>
if (f == null || f == '' || isNaN(f))
f = 0;
if (f === '' || isNaN(f))
f = null;

if (i == null || i == '' || isNaN(i) || i < 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't interval be zero also? I don't know for sure. I get the impression that setting it to zero should turn it off. Does that happen in the init scripts?

https://github.com/ppp-project/ppp/blob/616102e93be1c629821cafd83a89d9e6c0467033/pppd/lcp.c#L2492-L2493

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as failure is less than zero it should work.

https://github.com/openwrt/openwrt/blob/6720c4ccba256186bf2f1b1edadb851c447e62a5/package/network/services/ppp/files/ppp.sh#L133

This just means that setting it to zero disables it (in the init scipt) - which is inaccessible with thei < 1 behaviour. I'm not nit-picking your code- just what's going on here in general. That whole logic could do with a rewrite. Are you game?

@systemcrash
Copy link
Contributor

@feckert do you also use some flavour of PPP?

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.

LCP echo failure threshold 0 does not behave as described in LuCI
2 participants