Skip to content

Commit

Permalink
Adds back checking OF flows for CNI
Browse files Browse the repository at this point in the history
We see at scale that this can happen:
1. CNI delete
2. OVN is so busy it takes 30 seconds to remove the old logical port
3. CNI ADD within 30 seconds
4. ovn-controller sees old logical switchport, binds and considers new
   pod up, but no traffic works
5. sometime later OVN gets updated, and ovn-controller updates the pod
   with the new flows and traffic finally works

To solve this problem we need to have a minimal check to ensure the
right flows are present for the pod before we check if ovn_installed is
true. This change adds back the checks for mac address and of port
number.

Signed-off-by: Tim Rozet <trozet@redhat.com>
  • Loading branch information
trozet committed May 17, 2021
1 parent cae56d9 commit 22bed6a
Showing 1 changed file with 41 additions and 30 deletions.
71 changes: 41 additions & 30 deletions go-controller/pkg/cni/ovs.go
Expand Up @@ -167,28 +167,14 @@ func getIfaceOFPort(ifaceName string) (int, error) {
return iPort, nil
}

func doPodFlowsExist(mac string, ifAddrs []*net.IPNet, ofPort int) bool {
// Function checks for OpenFlow flows to know the pod is ready
// TODO(trozet): in the future use a more stable mechanism provided by OVN:
// https://bugzilla.redhat.com/show_bug.cgi?id=1839102

// query represents the match criteria, and different OF tables that this query may match on
type query struct {
match string
tables []int
}
type openflowQuery struct {
match string
tables []int
}

func getLegacyFlowQueries(mac string, ifAddrs []*net.IPNet, ofPort int) []openflowQuery {
// Query the flows by mac address for in_port_security and OF port
queries := []query{
{
match: "dl_src=" + mac,
tables: []int{9},
},
{
match: fmt.Sprintf("in_port=%d", ofPort),
tables: []int{0},
},
}
queries := getMinimalFlowQueries(mac, ofPort)
for _, ifAddr := range ifAddrs {
var ipMatch string
if !utilnet.IsIPv6(ifAddr.IP) {
Expand All @@ -200,9 +186,31 @@ func doPodFlowsExist(mac string, ifAddrs []*net.IPNet, ofPort int) bool {
// note we need to support table 48 for 20.06 OVN backwards compatibility. Table 49 is now
// where out_port_security lives
queries = append(queries,
query{fmt.Sprintf("%s=%s", ipMatch, ifAddr.IP), []int{48, 49}},
openflowQuery{fmt.Sprintf("%s=%s", ipMatch, ifAddr.IP), []int{48, 49}},
)
}
return queries
}

func getMinimalFlowQueries(mac string, ofPort int) []openflowQuery {
// Query the flows by mac address for in_port_security and OF port
queries := []openflowQuery{
{
match: "dl_src=" + mac,
tables: []int{9},
},
{
match: fmt.Sprintf("in_port=%d", ofPort),
tables: []int{0},
},
}
return queries
}

func doPodFlowsExist(queries []openflowQuery) bool {
// Function checks for OpenFlow flows to know the pod is ready
// TODO(trozet): in the future use a more stable mechanism provided by OVN:
// https://bugzilla.redhat.com/show_bug.cgi?id=1839102

// Must find the right flows in all queries to succeed
for _, query := range queries {
Expand All @@ -226,6 +234,12 @@ func doPodFlowsExist(mac string, ifAddrs []*net.IPNet, ofPort int) bool {
}

func waitForPodInterface(ctx context.Context, mac string, ifAddrs []*net.IPNet, ifaceName, ifaceID string, ofPort int, checkExternalIDs bool) error {
var queries []openflowQuery
if checkExternalIDs {
queries = getMinimalFlowQueries(mac, ofPort)
} else {
queries = getLegacyFlowQueries(mac, ifAddrs, ofPort)
}
timeout := time.After(20 * time.Second)
for {
select {
Expand All @@ -237,15 +251,12 @@ func waitForPodInterface(ctx context.Context, mac string, ifAddrs []*net.IPNet,
if err := isIfaceIDSet(ifaceName, ifaceID); err != nil {
return err
}

if checkExternalIDs {
if isIfaceOvnInstalledSet(ifaceName) {
//success
return nil
}
} else {
if doPodFlowsExist(mac, ifAddrs, ofPort) {
// success
if doPodFlowsExist(queries) {
if checkExternalIDs {
if isIfaceOvnInstalledSet(ifaceName) {
return nil
}
} else {
return nil
}
}
Expand Down

0 comments on commit 22bed6a

Please sign in to comment.