search

benoitc / erlang-nat

implements NAT handling facilities for Erlang applications
Other
57 stars 10 forks source link

Harmonize add_port_mapping #8

Closed macpie closed 6 years ago

macpie commented 6 years ago
  1. Harmonize add_port_mapping when LifeTime is 0.
  2. Set default lifetime to 0 (as mentioned in #5).
  3. Dialyzer fixes.
benoitc commented 6 years ago

Thanks for the patch!

My only issue with the change is not not use the recommanded lifetime for nat_pmp. They say in the specification:

The RECOMMENDED Port Mapping Lifetime is 3600 seconds.

Also the doc says :

A client can request the explicit deletion of all its UDP or TCP mappings by sending the same deletion request to the NAT gateway with external port, internal port, and lifetime set to 0.

So I would rather keep the default there. Maybe the lease should be resent every X Lifetime automatically and the library provide such mechanism?

macpie commented 6 years ago

I think we could make the default for all 3600 as long as 0 does not mean delete. Then if a user try to do a delete all (which is only for pmp I guess) he/she can use add_port_mapping(C, tcp, 0, 0, 0).

As for the tracking, it would required to transform nat into an app, personally I have decided to do the tracking in my app (libp2p).