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

Remove TUNNEL_MAP lookup #20170

Open
vincentmli opened this issue Jun 11, 2022 · 6 comments
Open

Remove TUNNEL_MAP lookup #20170

vincentmli opened this issue Jun 11, 2022 · 6 comments
Labels
pinned These issues are not marked stale by our issue bot. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.

Comments

@vincentmli
Copy link
Contributor

per discussion #17106 (comment), TUNNEL_MAP lookup is a fallback path for node pod cidr prefix, we could add the pod cidr prefix in IPCACHE and completely remove TUNNEL_MAP

@aanm aanm added the sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. label Jun 13, 2022
@vincentmli
Copy link
Contributor Author

the code to add pod cidr in tunnel_map

pkg/datapath/linux/node.go

  92 // updateTunnelMapping is called when a node update is received while running
  93 // with encapsulation mode enabled. The CIDR and IP of both the old and new
  94 // node are provided as context. The caller expects the tunnel mapping in the
  95 // datapath to be updated.
  96 func updateTunnelMapping(oldCIDR, newCIDR *cidr.CIDR, oldIP, newIP net.IP, firstAddition, encapEnabled bool, oldEncryptKey, newEnc     ryptKey uint8) {
  97         if !encapEnabled {
  98                 // When the protocol family is disabled, the initial node addition will
  99                 // trigger a deletion to clean up leftover entries. The deletion happens
 100                 // in quiet mode as we don't know whether it exists or not
 101                 if newCIDR != nil && firstAddition {
 102                         deleteTunnelMapping(newCIDR, true)
 103                 }
 104 
 105                 return
 106         }
 107 
 108         if cidrNodeMappingUpdateRequired(oldCIDR, newCIDR, oldIP, newIP, oldEncryptKey, newEncryptKey) {
 109                 log.WithFields(logrus.Fields{
 110                         logfields.IPAddr: newIP,
 111                         "allocCIDR":      newCIDR,
 112                 }).Debug("Updating tunnel map entry")
 113 
 114                 if err := tunnel.TunnelMap.SetTunnelEndpoint(newEncryptKey, newCIDR.IP, newIP); err != nil {
 115                         log.WithError(err).WithFields(logrus.Fields{
 116                                 "allocCIDR": newCIDR,
 117                         }).Error("bpf: Unable to update in tunnel endpoint map")
 118                 }
 119         }

it seems to be matter of replacing tunnel.TunnelMap.SetTunnelEndpoint with ipcache.Upsert, the question is what identity we should use for pod cidr prefix, @joestringer, do you have any suggestion on the identity?

@joestringer
Copy link
Member

With #19765 / #20117 we should be able to remove the requirement to specify the identity.

@vincentmli
Copy link
Contributor Author

ok, I will wait then till they are merged

@github-actions
Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 14, 2022
@vincentmli
Copy link
Contributor Author

no stale

@github-actions github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 15, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Oct 14, 2022
@joestringer joestringer added pinned These issues are not marked stale by our issue bot. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Oct 14, 2022
@gandro gandro mentioned this issue May 16, 2023
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned These issues are not marked stale by our issue bot. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

No branches or pull requests

3 participants