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.

Sign up for GitHub

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

Jump to bottom

datapath: support Hybrid-DSR mode for Geneve dispatch #25553

Merged
merged 2 commits into from
Jun 5, 2023
Merged

datapath: support Hybrid-DSR mode for Geneve dispatch #25553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b552c2e
datapath: allow DSR-Hybrid mode for Geneve-dispatch
julianwiedmann May 19, 2023
5bd8526
option: group NodePortModeHybrid with the other modes
julianwiedmann Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions daemon/cmd/kube_proxy_replacement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ func initKubeProxyReplacementOptions() error {
option.Config.LoadBalancerDSRDispatch != option.DSRDispatchIPIP &&
option.Config.LoadBalancerDSRDispatch != option.DSRDispatchGeneve ||
option.Config.NodePortMode == option.NodePortModeHybrid &&
option.Config.LoadBalancerDSRDispatch != option.DSRDispatchOption {
option.Config.LoadBalancerDSRDispatch != option.DSRDispatchOption &&
option.Config.LoadBalancerDSRDispatch != option.DSRDispatchGeneve {
return fmt.Errorf("Invalid value for --%s: %s", option.LoadBalancerDSRDispatch, option.Config.LoadBalancerDSRDispatch)
}

Expand Down Expand Up @@ -195,7 +196,7 @@ func initKubeProxyReplacementOptions() error {
option.Config.NodePortMode, option.Config.LoadBalancerDSRDispatch, option.Config.TunnelProtocol)
}

if option.Config.NodePortMode == option.NodePortModeDSR &&
if option.Config.NodePortMode != option.NodePortModeSNAT &&
option.Config.LoadBalancerDSRDispatch == option.DSRDispatchGeneve &&
option.Config.TunnelingEnabled() && option.Config.TunnelProtocol != option.TunnelGeneve {
return fmt.Errorf("Node Port %q mode with %s dispatch requires %s tunneling.",
Expand Down
125 changes: 125 additions & 0 deletions daemon/cmd/kube_proxy_replacement_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type kprConfig struct {
routingMode string
tunnelProtocol string
nodePortMode string
dispatchMode string
}

func (cfg *kprConfig) set() {
Expand All @@ -52,6 +53,7 @@ func (cfg *kprConfig) set() {
option.Config.EnableSocketLBTracing = true
option.Config.RoutingMode = cfg.routingMode
option.Config.TunnelProtocol = cfg.tunnelProtocol
option.Config.LoadBalancerDSRDispatch = cfg.dispatchMode

if cfg.nodePortMode == option.NodePortModeDSR || cfg.nodePortMode == option.NodePortModeHybrid {
option.Config.NodePortMode = cfg.nodePortMode
Expand Down Expand Up @@ -232,6 +234,129 @@ func (s *KPRSuite) TestInitKubeProxyReplacementOptions(c *C) {
cfg.routingMode = option.RoutingModeTunnel
cfg.tunnelProtocol = option.TunnelVXLAN
cfg.nodePortMode = option.NodePortModeDSR
cfg.dispatchMode = option.DSRDispatchOption
},
kprConfig{
expectedErrorRegex: "Node Port .+ mode cannot be used with .+ tunneling.",
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},

// Node port DSR mode + Geneve dispatch + native routing
{
"node-port-dsr-mode+geneve-dispatch+native-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeNative
cfg.nodePortMode = option.NodePortModeDSR
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},
// Node port DSR mode + Geneve dispatch + geneve routing
{
"node-port-dsr-mode+geneve-dispatch+geneve-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeTunnel
cfg.tunnelProtocol = option.TunnelGeneve
cfg.nodePortMode = option.NodePortModeDSR
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},
// Node port DSR mode + Geneve dispatch + vxlan routing: error as they're incompatible
{
"node-port-dsr-mode+geneve-dispatch+vxlan-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeTunnel
cfg.tunnelProtocol = option.TunnelVXLAN
cfg.nodePortMode = option.NodePortModeDSR
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
expectedErrorRegex: "Node Port .+ mode cannot be used with .+ tunneling.",
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},

// Node port Hybrid mode + Geneve dispatch + native routing
{
"node-port-hybrid-mode+geneve-dispatch+native-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeNative
cfg.nodePortMode = option.NodePortModeHybrid
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},
// Node port Hybrid mode + Geneve dispatch + geneve routing
{
"node-port-hybrid-mode+geneve-dispatch+geneve-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeTunnel
cfg.tunnelProtocol = option.TunnelGeneve
cfg.nodePortMode = option.NodePortModeHybrid
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
enableSocketLB: true,
enableNodePort: true,
enableHostPort: true,
enableExternalIPs: true,
enableSessionAffinity: true,
enableHostLegacyRouting: false,
enableSocketLBTracing: true,
},
},
// Node port Hybrid mode + Geneve dispatch + vxlan routing: error as they're incompatible
{
"node-port-hybrid-mode+geneve-dispatch+vxlan-routing",
func(cfg *kprConfig) {
cfg.kubeProxyReplacement = option.KubeProxyReplacementStrict
cfg.routingMode = option.RoutingModeTunnel
cfg.tunnelProtocol = option.TunnelVXLAN
cfg.nodePortMode = option.NodePortModeHybrid
cfg.dispatchMode = option.DSRDispatchGeneve
},
kprConfig{
expectedErrorRegex: "Node Port .+ mode cannot be used with .+ tunneling.",
Expand Down
2 changes: 1 addition & 1 deletion pkg/datapath/linux/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (h *HeaderfileWriter) WriteNodeConfig(w io.Writer, cfg *datapath.LocalNodeC
encapProto := option.Config.TunnelProtocol
if !option.Config.TunnelingEnabled() &&
option.Config.EnableNodePort &&
option.Config.NodePortMode == option.NodePortModeDSR &&
option.Config.NodePortMode != option.NodePortModeSNAT &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more concise than == DSR || == Hybrid but maybe less future proof?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, not a big fan of it either - for now, I believe this is in sync with how the remaining code does it. But I can follow up with some NodePortUsesDSR() helper.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. If we are going to make setting more logical, I also vote for not using the name NodePort to refer to everything related to forwarding traffic

option.Config.LoadBalancerDSRDispatch == option.DSRDispatchGeneve {
encapProto = option.TunnelGeneve
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/datapath/loader/base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ func (l *Loader) Reinitialize(ctx context.Context, o datapath.BaseProgramOwner,

if !option.Config.TunnelingEnabled() &&
option.Config.EnableNodePort &&
option.Config.NodePortMode == option.NodePortModeDSR &&
option.Config.NodePortMode != option.NodePortModeSNAT &&
option.Config.LoadBalancerDSRDispatch == option.DSRDispatchGeneve {
encapProto = option.TunnelGeneve
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/option/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,15 +1261,15 @@ const (
// NodePortModeDSR is for performing DSR for requests to remote nodes
NodePortModeDSR = "dsr"

// NodePortModeHybrid is a dual mode of the above, that is, DSR for TCP and SNAT for UDP
NodePortModeHybrid = "hybrid"

// NodePortAlgRandom is for randomly selecting a backend
NodePortAlgRandom = "random"

// NodePortAlgMaglev is for using maglev consistent hashing for backend selection
NodePortAlgMaglev = "maglev"

// NodePortModeHybrid is a dual mode of the above, that is, DSR for TCP and SNAT for UDP
NodePortModeHybrid = "hybrid"

// DSR dispatch mode to encode service into IP option or extension header
DSRDispatchOption = "opt"

Expand Down Expand Up @@ -3136,7 +3136,7 @@ func (c *DaemonConfig) Populate(vp *viper.Viper) {
// manually pick port for native-routing and DSR with Geneve dispatch:
if !c.TunnelingEnabled() &&
(c.EnableNodePort || c.KubeProxyReplacement == KubeProxyReplacementStrict) &&
c.NodePortMode == NodePortModeDSR &&
c.NodePortMode != NodePortModeSNAT &&
c.LoadBalancerDSRDispatch == DSRDispatchGeneve {
c.TunnelPort = defaults.TunnelPortGeneve
} else {
Expand Down
25 changes: 25 additions & 0 deletions test/k8s/services.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,19 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`,
testNodePortExternal(kubectl, ni, false, true, true)
})

It("Tests with TC, direct routing and Hybrid-DSR with Geneve", func() {
DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, map[string]string{
"loadBalancer.acceleration": "disabled",
"loadBalancer.mode": "hybrid",
"loadBalancer.algorithm": "random",
"routingMode": "native",
"autoDirectNodeRoutes": "true",
"loadBalancer.dsrDispatch": "geneve",
"devices": fmt.Sprintf(`'{}'`),
})
testNodePortExternal(kubectl, ni, false, true, false)
})

It("Tests with TC, geneve tunnel, dsr and Maglev", func() {
DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, map[string]string{
"loadBalancer.acceleration": "disabled",
Expand All @@ -602,6 +615,18 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`,
testNodePortExternal(kubectl, ni, false, true, true)
})

It("Tests with TC, geneve tunnel, and Hybrid-DSR with Geneve", func() {
DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, map[string]string{
"loadBalancer.acceleration": "disabled",
"loadBalancer.mode": "hybrid",
"loadBalancer.algorithm": "random",
"tunnelProtocol": "geneve",
"loadBalancer.dsrDispatch": "geneve",
"devices": fmt.Sprintf(`'{}'`),
})
testNodePortExternal(kubectl, ni, false, true, false)
})

// Run on net-next and 4.19 but not on old versions, because of
// LRU requirement.
SkipItIf(helpers.DoesNotRunOn419OrLaterKernel, "Supports IPv4 fragments", func() {
Expand Down