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

Kill multicast rate limits inside ZT -- no longer needed, uses memory and CPU #191

Closed
mwarning opened this issue Jun 22, 2015 · 14 comments
Closed

Comments

@mwarning
Copy link
Contributor

Entries from map _multicastRateAccounts in node/Network.cpp do not seem to be removed anywhere.

@adamierymenko
Copy link
Contributor

I've considered removing multicast rate limits, since it does sender-size replication. The sender incurs all bandwidth for multicast and if someone is screaming multicast on your network that's a problem with that participant.

@mwarning
Copy link
Contributor Author

Yes, I agree. It would also save a lot of resources.

@adamierymenko
Copy link
Contributor

Yeah, let's do it. It's kind of left over from the old peer to peer distributed graph traversal algorithm, which I dumped for sender-side replication for various reasons. Simple is good.

@adamierymenko adamierymenko changed the title structural memory leak in Network.cpp Kill multicast rate limits Jun 22, 2015
@adamierymenko adamierymenko changed the title Kill multicast rate limits Kill multicast rate limits -- no longer needed, uses memory and CPU Jun 22, 2015
@adamierymenko
Copy link
Contributor

I actually like deleting code. Feels good. :)

@adamierymenko
Copy link
Contributor

Let me do this one -- it touches a few things.

@mwarning
Copy link
Contributor Author

So ... relieving, I was about to make a pull request. ;-)

@keesbos
Copy link
Contributor

keesbos commented Jun 22, 2015

:-) And multicast can still be prohibited by filters (netflow). Or if you have an reasonable firewall implementation, rate limiting can be done too by the firewall.

@adamierymenko
Copy link
Contributor

Yeah -- thing is, with sender-side replication the rate limits are not enforced except by the sender... so there's no security against flooding benefit here. That and a flooder pays all bandwidth, so there is no amplification attack.

In the old multicast propagation algorithm I had this rate limiting algorithm to prevent amplification attacks, but the whole thing was too complex/expensive compared to simple sender-side replication.

@heri16
Copy link

heri16 commented Jun 24, 2015

Please clarify the title that we are talking about multicast INSIDE a zerotier network.

@mwarning mwarning changed the title Kill multicast rate limits -- no longer needed, uses memory and CPU Kill multicast rate limits inside ZT -- no longer needed, uses memory and CPU Jun 24, 2015
@adamierymenko
Copy link
Contributor

Yes, multicast inside a network.

@adamierymenko
Copy link
Contributor

Not sure what other kind there would be.

adamierymenko added a commit that referenced this issue Jun 26, 2015
…ere not well supported or easily configurable anyway) -- this is really left over from the old collaborative multicast propagation algorithm. New algorithm (in for a while) has been sender-side replication in which sender "pays" all bandwidth, which intrinsically limits multicast.
@heri16
Copy link

heri16 commented Jun 27, 2015

The Bounjour Discovery Protocol for nodes to discover each other on a local private network uses multicast instead of broadcasts, and works better in most kinds of multi-nat or multi-router scenarios.

@heri16
Copy link

heri16 commented Dec 16, 2015

May I ask, why is multicastLimit still inside the api for network controller configuration, if multicastLimit now means nothing?

@adamierymenko
Copy link
Contributor

Different concept. The current multicastLimit is the maximum number of recipients. This was the maximum transfer rate.

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

No branches or pull requests

4 participants