Skip to content
5 changes: 3 additions & 2 deletions go-controller/pkg/libovsdb/ops/db_object_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,10 @@ var ACLNetworkPolicyPortIndex = newObjectIDsType(acl, NetworkPolicyPortIndexOwne
// ingress/egress + NetworkPolicy[In/E]gressRule idx - defines given gressPolicy.
// ACLs are created for gp.portPolicies which are grouped by protocol:
// - for empty policy (no selectors and no ip blocks) - empty ACL (see allIPsMatch)
// with idx=emptyIdx (-1)
// OR
// - all selector-based peers ACL
// - for every IPBlock +1 ACL
// - all selector-based peers ACL with idx=emptyIdx (-1)
// - all ipBlocks combined into a single ACL with idx=ipBlockCombinedIdx (-2)
// Therefore unique id for a given gressPolicy is protocol name + IPBlock idx
// (protocol will be "None" if no port policy is defined, and empty policy and all
// selector-based peers ACLs will have idx=-1)
Expand Down
48 changes: 28 additions & 20 deletions go-controller/pkg/ovn/gress_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import (
const (
// emptyIdx is used to create ACL for gressPolicy that doesn't have ipBlocks
emptyIdx = -1
// ipBlockCombinedIdx is used when creating an ACL for a gressPolicy
// that contains ipBlocks. Previously, one ACL was created per ipBlock.
// This is changed to create a single combined ACL for all ipBlocks,
// and this special index value identifies those new ACLs.
ipBlockCombinedIdx = -2
)

type gressPolicy struct {
Expand Down Expand Up @@ -167,14 +172,14 @@ func (gp *gressPolicy) allIPsMatch() string {
}
}

func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) []string {
func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) string {
var direction string
if gp.policyType == knet.PolicyTypeIngress {
direction = "src"
} else {
direction = "dst"
}
var matchStrings []string
var ipBlockMatches []string
var matchStr, ipVersion string
for _, ipBlock := range gp.ipBlocks {
if utilnet.IsIPv6CIDRString(ipBlock.CIDR) {
Expand All @@ -185,17 +190,22 @@ func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) []string
if len(ipBlock.Except) == 0 {
matchStr = fmt.Sprintf("%s.%s == %s", ipVersion, direction, ipBlock.CIDR)
} else {
matchStr = fmt.Sprintf("%s.%s == %s && %s.%s != {%s}", ipVersion, direction, ipBlock.CIDR,
matchStr = fmt.Sprintf("(%s.%s == %s && %s.%s != {%s})", ipVersion, direction, ipBlock.CIDR,
ipVersion, direction, strings.Join(ipBlock.Except, ", "))
}
if l4Match == libovsdbutil.UnspecifiedL4Match {
matchStr = fmt.Sprintf("%s && %s", matchStr, lportMatch)
} else {
matchStr = fmt.Sprintf("%s && %s && %s", matchStr, l4Match, lportMatch)
}
matchStrings = append(matchStrings, matchStr)
ipBlockMatches = append(ipBlockMatches, matchStr)
}
return matchStrings
var l3Match string
if len(ipBlockMatches) == 1 {
l3Match = ipBlockMatches[0]
} else {
l3Match = fmt.Sprintf("(%s)", strings.Join(ipBlockMatches, " || "))
}

if l4Match == libovsdbutil.UnspecifiedL4Match {
return fmt.Sprintf("%s && %s", l3Match, lportMatch)
}
return fmt.Sprintf("%s && %s && %s", l3Match, l4Match, lportMatch)
}

// addNamespaceAddressSet adds a namespace address set to the gress policy.
Expand Down Expand Up @@ -285,13 +295,11 @@ func (gp *gressPolicy) buildLocalPodACLs(portGroupName string, aclLogging *libov
for protocol, l4Match := range libovsdbutil.GetL4MatchesFromNetworkPolicyPorts(gp.portPolicies) {
if len(gp.ipBlocks) > 0 {
// Add ACL allow rule for IPBlock CIDR
ipBlockMatches := gp.getMatchFromIPBlock(lportMatch, l4Match)
for ipBlockIdx, ipBlockMatch := range ipBlockMatches {
aclIDs := gp.getNetpolACLDbIDs(ipBlockIdx, protocol)
acl := libovsdbutil.BuildACLWithDefaultTier(aclIDs, types.DefaultAllowPriority, ipBlockMatch, action,
aclLogging, gp.aclPipeline)
createdACLs = append(createdACLs, acl)
}
ipBlockMatch := gp.getMatchFromIPBlock(lportMatch, l4Match)
aclIDs := gp.getNetpolACLDbIDs(ipBlockCombinedIdx, protocol)
acl := libovsdbutil.BuildACLWithDefaultTier(aclIDs, types.DefaultAllowPriority, ipBlockMatch, action,
aclLogging, gp.aclPipeline)
createdACLs = append(createdACLs, acl)
}
// if there are pod/namespace selector, then allow packets from/to that address_set or
// if the NetworkPolicyPeer is empty, then allow from all sources or to all destinations.
Expand Down Expand Up @@ -334,10 +342,10 @@ func (gp *gressPolicy) getNetpolACLDbIDs(ipBlockIdx int, protocol string) *libov
// gress rule index
libovsdbops.GressIdxKey: strconv.Itoa(gp.idx),
// acls are created for every gp.portPolicies which are grouped by protocol:
// - for empty policy (no selectors and no ip blocks) - empty ACL
// - for empty policy (no selectors and no ip blocks) - empty ACL with idx=emptyIdx (-1)
// OR
// - all selector-based peers ACL
// - for every IPBlock +1 ACL
// - all selector-based peers ACL with idx=emptyIdx (-1)
// - all ipBlocks combined into a single ACL with idx=ipBlockCombinedIdx (-2)
// Therefore unique id for a given gressPolicy is protocol name + IPBlock idx
// (protocol will be "None" if no port policy is defined, and empty policy and all
// selector-based peers ACLs will have idx=-1)
Expand Down
19 changes: 8 additions & 11 deletions go-controller/pkg/ovn/gress_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
ipBlocks []*knet.IPBlock
lportMatch string
l4Match string
expected []string
expected string
}{
{
desc: "IPv4 only no except",
Expand All @@ -27,7 +27,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip4.src == 0.0.0.0/0 && input && fake"},
expected: "ip4.src == 0.0.0.0/0 && input && fake",
},
{
desc: "multiple IPv4 only no except",
Expand All @@ -41,8 +41,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip4.src == 0.0.0.0/0 && input && fake",
"ip4.src == 10.1.0.0/16 && input && fake"},
expected: "(ip4.src == 0.0.0.0/0 || ip4.src == 10.1.0.0/16) && input && fake",
},
{
desc: "IPv6 only no except",
Expand All @@ -53,7 +52,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip6.src == fd00:10:244:3::49/32 && input && fake"},
expected: "ip6.src == fd00:10:244:3::49/32 && input && fake",
},
{
desc: "mixed IPv4 and IPv6 no except",
Expand All @@ -67,8 +66,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip6.src == ::/0 && input && fake",
"ip4.src == 0.0.0.0/0 && input && fake"},
expected: "(ip6.src == ::/0 || ip4.src == 0.0.0.0/0) && input && fake",
},
{
desc: "IPv4 only with except",
Expand All @@ -80,7 +78,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake"},
expected: "(ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16}) && input && fake",
},
{
desc: "multiple IPv4 with except",
Expand All @@ -95,8 +93,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake",
"ip4.src == 10.1.0.0/16 && input && fake"},
expected: "((ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16}) || ip4.src == 10.1.0.0/16) && input && fake",
},
{
desc: "IPv4 with IPv4 except",
Expand All @@ -108,7 +105,7 @@ func TestGetMatchFromIPBlock(t *testing.T) {
},
lportMatch: "fake",
l4Match: "input",
expected: []string{"ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16} && input && fake"},
expected: "(ip4.src == 0.0.0.0/0 && ip4.src != {10.1.0.0/16}) && input && fake",
},
}

Expand Down
Loading