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

Send broadcast messages to all interfaces #38

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

Conversation

aikar
Copy link
Contributor

@aikar aikar commented Feb 7, 2020

Look up all available interfaces and build an array of destinations for each for broadcast.

Resolves a TODO you had here.

Used a library to implement the logic of turning an address and netmask into a broadcast address.

Have tested this locally where I have 2 ifaces available (wireless and wired), another machine only had 1 (wireless)
Previously had issues where my outbound packets only went out wired, so the wireless client could not receive mine, however I could receive the wireless packets as I was also on wireless.

This resolves the issue by sending my outbound over both wired and wireless.

Resolves #26

Look up all available interfaces and build an array of destinations for each for broadcast.
@wankdanker
Copy link
Owner

Awesome. One thought I have is if options.address has been specified, then we should probably only resolve the broadcast address for the interface that has that address. Reason being, if I am specifying the address I want things bound to, I probably don't want packets broadcast on all interfaces.

It is possible that there is not an interface with the specified address though (sysctl -w net.ipv4.ip_nonlocal_bind=1), so, in that case I'd probably just broadcast to all interfaces.

@@ -96,7 +97,29 @@ Network.prototype.start = function (callback) {
self.socket.setBroadcast(true);

//TODO: get the default broadcast address from os.networkInterfaces() (not currently returned)
self.destination = [self.broadcast || "255.255.255.255"];

if (self.broadcast) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wankdanker this handles if the user DOES manually define broadcast as an option, to use what they supplied. Also expands the API to support passing an array instead of a single value.

The new code below only activates if the user did not manually declare broadcast.

@aikar
Copy link
Contributor Author

aikar commented Feb 8, 2020

Oh, to follow on that feedback i noticed you said address now and not broadcast.

I can see what you mean now. Not sure best way to implement that though.

I assume would have to compare address to cidr/netmask to compare?

Are you able to implement those changes? (you do have push to my branch for this PR)

@YindSoft
Copy link

Hi!, are you merging this fix? because I'm having the same issue, my node connects to the other nodes an receives data but those nodes doesn't know about mine and I can't send data. I change the code with this PR and it worked.

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.

.advertise network interface binding?
3 participants