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

mwan3: Add support for nslookup track method #25676

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

brianjmurrell
Copy link
Contributor

Maintainer: @feckert
Compile tested: x86_64, 23.05.5

Description:
In cases where ICMP ECHO is blocked by the ISP or perhaps even just de-prioritized, there can be a lot of ping failures when the link is perfectly fine. In such cases, doing a DNS query is much more reliable so add a link-test method to use nslookup to do a DNS query.

Also, fix a couple of small bugs in the nping method.

@feckert
Copy link
Member

feckert commented Jan 8, 2025

Please bump the PKG_VERSION with this commit so we have a new Version 2.11.17.

@brianjmurrell
Copy link
Contributor Author

Please bump the PKG_VERSION with this commit so we have a new Version 2.11.17.

Sure thing. Do/can you squash on merging or do you want it squashed here before you merge?

@feckert
Copy link
Member

feckert commented Jan 8, 2025

Please make sure we only have one commit. Then I just have to press merge.

@@ -322,11 +328,17 @@ main() {
fi
;;
nping-*)
WRAP nping -c $count $track_ip --${FAMILY#nping-} > $TRACK_OUTPUT &
WRAP nping -${FAMILY#ipv} -c $count $track_ip --${track_method#nping-} > $TRACK_OUTPUT &
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that. That must be moved to a separate commit with a description of what you are fixing there

Copy link
Contributor Author

@brianjmurrell brianjmurrell Jan 9, 2025

Choose a reason for hiding this comment

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

Do you mean separate commit in this PR or seperate/new PR?

Copy link
Member

Choose a reason for hiding this comment

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

You could add this to this PR. But in a separate commit.

The IP address family is a single dash option and one of 4 or 6, not tcp
or udp.

Signed-off-by: Brian J. Murrell <[email protected]>
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.

2 participants