Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions pkg/daemon/controller_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type LbServiceRules struct {
BridgeName string
DstMac string
UnderlayNic string
SubnetName string
}

func evalCommandSymlinks(cmd string) (string, error) {
Expand Down Expand Up @@ -412,7 +413,7 @@ func (c *Controller) reconcileRouters(event *subnetEvent) error {
return nil
}

func genLBServiceRules(service *v1.Service, bridgeName, underlayNic, dstMac string) []LbServiceRules {
func genLBServiceRules(service *v1.Service, bridgeName, underlayNic, dstMac, subnetName string) []LbServiceRules {
var lbServiceRules []LbServiceRules
for _, ingress := range service.Status.LoadBalancer.Ingress {
for _, port := range service.Spec.Ports {
Expand All @@ -423,6 +424,7 @@ func genLBServiceRules(service *v1.Service, bridgeName, underlayNic, dstMac stri
DstMac: dstMac,
UnderlayNic: underlayNic,
BridgeName: bridgeName,
SubnetName: subnetName,
})
}
}
Expand All @@ -433,22 +435,24 @@ func (c *Controller) diffExternalLBServiceRules(oldService, newService *v1.Servi
var oldlbServiceRules, newlbServiceRules []LbServiceRules

if oldService != nil && oldService.Annotations[util.ServiceExternalIPFromSubnetAnnotation] != "" {
oldBridgeName, underlayNic, dstMac, err := c.getExtInfoBySubnet(oldService.Annotations[util.ServiceExternalIPFromSubnetAnnotation])
oldSubnetName := oldService.Annotations[util.ServiceExternalIPFromSubnetAnnotation]
oldBridgeName, underlayNic, dstMac, err := c.getExtInfoBySubnet(oldSubnetName)
if err != nil {
klog.Errorf("failed to get provider network by subnet %s: %v", oldService.Annotations[util.ServiceExternalIPFromSubnetAnnotation], err)
klog.Errorf("failed to get provider network by subnet %s: %v", oldSubnetName, err)
return nil, nil, err
}

oldlbServiceRules = genLBServiceRules(oldService, oldBridgeName, underlayNic, dstMac)
oldlbServiceRules = genLBServiceRules(oldService, oldBridgeName, underlayNic, dstMac, oldSubnetName)
}

if isSubnetExternalLBEnabled && newService != nil && newService.Annotations[util.ServiceExternalIPFromSubnetAnnotation] != "" {
newBridgeName, underlayNic, dstMac, err := c.getExtInfoBySubnet(newService.Annotations[util.ServiceExternalIPFromSubnetAnnotation])
newSubnetName := newService.Annotations[util.ServiceExternalIPFromSubnetAnnotation]
newBridgeName, underlayNic, dstMac, err := c.getExtInfoBySubnet(newSubnetName)
if err != nil {
klog.Errorf("failed to get provider network by subnet %s: %v", newService.Annotations[util.ServiceExternalIPFromSubnetAnnotation], err)
klog.Errorf("failed to get provider network by subnet %s: %v", newSubnetName, err)
return nil, nil, err
}
newlbServiceRules = genLBServiceRules(newService, newBridgeName, underlayNic, dstMac)
newlbServiceRules = genLBServiceRules(newService, newBridgeName, underlayNic, dstMac, newSubnetName)
}

for _, oldRule := range oldlbServiceRules {
Expand Down Expand Up @@ -557,7 +561,7 @@ func (c *Controller) reconcileServices(event *serviceEvent) error {
if len(lbServiceRulesToAdd) > 0 {
for _, rule := range lbServiceRulesToAdd {
klog.Infof("Adding LB service rule: %+v", rule)
if err := c.AddOrUpdateUnderlaySubnetSvcLocalFlowCache(rule.IP, rule.Port, rule.Protocol, rule.DstMac, rule.UnderlayNic, rule.BridgeName); err != nil {
if err := c.AddOrUpdateUnderlaySubnetSvcLocalFlowCache(rule.IP, rule.Port, rule.Protocol, rule.DstMac, rule.UnderlayNic, rule.BridgeName, rule.SubnetName); err != nil {
klog.Errorf("failed to update underlay subnet svc local openflow cache: %v", err)
return err
}
Expand Down
22 changes: 13 additions & 9 deletions pkg/daemon/flow_rules_linux.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
package daemon

import (
"errors"
"fmt"
"strconv"
"strings"

"k8s.io/klog/v2"

kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
"github.com/kubeovn/kube-ovn/pkg/ovs"
"github.com/kubeovn/kube-ovn/pkg/util"
)

const flowKindUnderlayService = "usvc"

func (c *Controller) AddOrUpdateUnderlaySubnetSvcLocalFlowCache(serviceIP string, port uint16, protocol, dstMac, underlayNic, bridgeName string) error {
func (c *Controller) AddOrUpdateUnderlaySubnetSvcLocalFlowCache(serviceIP string, port uint16, protocol, dstMac, underlayNic, bridgeName, subnetName string) error {
inPort, err := c.getPortID(bridgeName, underlayNic)
if err != nil {
return err
}

outPort, err := c.getPortID(bridgeName, "patch-localnet.")
patchPortName := fmt.Sprintf("patch-localnet.%s-to-br-int", subnetName)
outPort, err := c.getPortID(bridgeName, patchPortName)
if err != nil {
klog.V(5).Infof("patch-localnet port not found on bridge %s, skipping underlay service flow for %s:%d (no pods on this node yet)", bridgeName, serviceIP, port)
klog.V(5).Infof("patch-localnet port %s not found on bridge %s, skipping underlay service flow for %s:%d (subnet %s may not have pods on this node yet)", patchPortName, bridgeName, serviceIP, port, subnetName)
return nil
}

Expand Down Expand Up @@ -77,14 +79,16 @@
return fmt.Sprintf("%s-%s-%s-%d-%s", kind, ip, protocol, port, extra)
}

func (c *Controller) getPortID(bridgeName, portName string) (int, error) {

Check failure on line 82 in pkg/daemon/flow_rules_linux.go

View workflow job for this annotation

GitHub Actions / Build kube-ovn

unused-parameter: parameter 'bridgeName' seems to be unused, consider removing or renaming it as _ (revive)
if c.ovsClient == nil {
return 0, errors.New("ovs client not initialized")
ofportStr, err := ovs.Get("Interface", portName, "ofport", "", true)
if err != nil {
return 0, fmt.Errorf("failed to get ofport for interface %s: %w", portName, err)
}

portInfo, err := c.ovsClient.OpenFlow.DumpPort(bridgeName, portName)
portID, err := strconv.Atoi(strings.TrimSpace(ofportStr))
if err != nil {
return 0, fmt.Errorf("failed to dump port %s on bridge %s: %w", portName, bridgeName, err)
return 0, fmt.Errorf("failed to parse ofport %q: %w", ofportStr, err)
}
return portInfo.PortID, nil

return portID, nil
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The bridgeName parameter is unused in this function after the refactoring. It should be removed from the function signature and its call sites in AddOrUpdateUnderlaySubnetSvcLocalFlowCache to improve code clarity.

Suggested change
func (c *Controller) getPortID(bridgeName, portName string) (int, error) {
if c.ovsClient == nil {
return 0, errors.New("ovs client not initialized")
ofportStr, err := ovs.Get("Interface", portName, "ofport", "", true)
if err != nil {
return 0, fmt.Errorf("failed to get ofport for interface %s: %w", portName, err)
}
portInfo, err := c.ovsClient.OpenFlow.DumpPort(bridgeName, portName)
portID, err := strconv.Atoi(strings.TrimSpace(ofportStr))
if err != nil {
return 0, fmt.Errorf("failed to dump port %s on bridge %s: %w", portName, bridgeName, err)
return 0, fmt.Errorf("failed to parse ofport %q: %w", ofportStr, err)
}
return portInfo.PortID, nil
return portID, nil
}
func (c *Controller) getPortID(portName string) (int, error) {
oftportStr, err := ovs.Get("Interface", portName, "ofport", "", true)
if err != nil {
return 0, fmt.Errorf("failed to get ofport for interface %s: %w", portName, err)
}
portID, err := strconv.Atoi(strings.TrimSpace(ofportStr))
if err != nil {
return 0, fmt.Errorf("failed to parse ofport %q: %w", ofportStr, err)
}
return portID, nil
}

Loading