-
Notifications
You must be signed in to change notification settings - Fork 881
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
Few changes in encryption overlay #1354
Conversation
- Current code programs src/dst cidr like 192.168.100.126/128 Signed-off-by: Alessandro Boch <[email protected]>
@@ -20,7 +20,7 @@ import ( | |||
) | |||
|
|||
const ( | |||
mark = uint32(0xD0C4E3) | |||
r = 0xD0C4E3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why r
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a simply a scalar, that the code use for creating two independent entities: a mark and a request id.
It has no meaning by itself, so I chose the first one char variable name it came to my mind.
drivers/overlay/encryption.go
Outdated
@@ -31,6 +31,12 @@ const ( | |||
bidir | |||
) | |||
|
|||
var ( | |||
saTxHardLimit = netlink.XfrmStateLimits{TimeHard: uint64(12*3600 + 60)} | |||
saRxHardLimit = netlink.XfrmStateLimits{TimeHard: uint64(3*12*3600 + 60)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will these hardlimits kick in if there are no traffic on those encrypted tunnels ?
If yes, and if the hardlimit kicks in, how will the entries be reestablished ?
(in other words, is it true that the entries will be removed only if the daemon is down - as mentioned in the PR description ? )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are hard limits, so independent of traffic hit.
Yes, they will kick in if the daemon is down or if no more encrypted networks are present.
If an encrypted network is present, the hard timeout will never kick in, because we replace the entries with new ones (at key rotation) before the timers expire.
@@ -237,9 +243,11 @@ func programSA(localIP, remoteIP net.IP, spi *spi, k *key, dir int, add bool) (f | |||
Proto: netlink.XFRM_PROTO_ESP, | |||
Spi: spi.reverse, | |||
Mode: netlink.XFRM_MODE_TRANSPORT, | |||
Reqid: r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you pls explain why this is required ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is required to label the SAs that are programmed by us, so that we can remove only those when we do the cleanup and not disrupt any existing one on the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding this fix!
- Use the request id for labelling our SAs Signed-off-by: Alessandro Boch <[email protected]>
Related to moby/moby/issues/30727
Signed-off-by: Alessandro Boch [email protected]