Added TTL option for tunnels (LP: #1846783) #139
Conversation
Codecov Report
@@ Coverage Diff @@
## master #139 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 39 39
Lines 5453 5453
=========================================
Hits 5453 5453 Continue to review full report at Codecov.
|
|
Hey, thank you for your contribution. |
There was a problem hiding this comment.
Thank you very much for your contribution. I've added some inline comments for smaller issues with the code. Could you please have a look at them?
Also, the test suite seems to be failing, as not all test have been adopted to the new TTL setting. Could you please check the 21 failures and adopt those test cases accordingly? It should be just adding the TTL=64/ttl=64 values for networkd/NetworkManager to the correct places. So that we can run make check successfully.
Failing tests: https://paste.ubuntu.com/p/ffByVxcC3b/
doc/netplan.md
Outdated
| @@ -967,6 +967,10 @@ more general information about tunnels. | |||
|
|
|||
| : Defines the address of the remote endpoint of the tunnel. | |||
|
|
|||
| ``ttl`` (scalar) | |||
There was a problem hiding this comment.
Could you please append – since **0.100** to this line, so we can document that this setting will be introduced with the current development version (0.100)
src/parse.h
Outdated
| @@ -329,6 +329,7 @@ struct net_definition { | |||
| char *remote_ip; | |||
| char *input_key; | |||
| char *output_key; | |||
| guint ttl; | |||
There was a problem hiding this comment.
Please fix the indentation to be in line with the other variables of this struct (8 spaces).
src/validation.c
Outdated
| @@ -72,6 +72,11 @@ validate_tunnel_grammar(NetplanNetDefinition* nd, yaml_node_t* node, GError** er | |||
| if (!nd->tunnel.remote_ip) | |||
| return yaml_error(node, error, "%s: missing 'remote' property for tunnel", nd->id); | |||
|
|
|||
| if (nd->tunnel.ttl) { | |||
| if ((nd->tunnel.ttl) < 1 || (nd->tunnel.ttl) > 255) | |||
There was a problem hiding this comment.
I think we can combine the two ifs.. if (nd->tunnel.ttl) is true it will be > 0 (as this is a uint variable), which makes the check for nd->tunnel.ttl < 1 useles.
So we could just combine it as: if (nd->tunnel.ttl && nd->tunnel.ttl > 255)
|
This would close https://bugs.launchpad.net/netplan/+bug/1846783 |
slyon
changed the title
|
According to https://bugs.launchpad.net/netplan/+bug/1846783 this is required for IPIP/SIT/GRE tunnels. |
|
This PR was now merged. Thank you very much for your contribution @kev1989 ! |
Description
Some protocols set the TTL field of the packet to 1; when passing through the tunnel, the packet is discarded. To solve the problem, the tunnel has the TTL option, but it was not in netplan.
Checklist
make checksuccessfully.make check-coverage).