Skip to content

Commit

Permalink
Merge pull request projectcalico#2609 from tomastigera/tomas-bpf-polp…
Browse files Browse the repository at this point in the history 
…rog-structs

BPF policy code reshuffle for easier maintainability
  • Loading branch information
@tomastigera
tomastigera authored Dec 14, 2020
2 parents 0d79f37 + dad445b commit 94ca599
Show file tree
Hide file tree
Showing 7 changed files with 434 additions and 229 deletions.
88 changes: 63 additions & 25 deletions bpf/polprog/pol_prog_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
log "github.com/sirupsen/logrus"

. "github.com/projectcalico/felix/bpf/asm"
"github.com/projectcalico/felix/bpf/state"
"github.com/projectcalico/felix/ip"
"github.com/projectcalico/felix/proto"
"github.com/projectcalico/felix/rules"
Expand Down Expand Up @@ -103,7 +104,26 @@ var (
ipsKeyPad int16 = 19
)

func (p *Builder) Instructions(rules [][][]*proto.Rule) (Insns, error) {
type Rule = *proto.Rule

type Policy struct {
Name string
Rules []Rule
}

type Tier struct {
Name string
Policies []Policy
}

type Rules struct {
Tiers []Tier
Profiles []Profile
}

type Profile = Policy

func (p *Builder) Instructions(rules Rules) (Insns, error) {
p.b = NewBlock()
p.writeProgramHeader()
p.writeRules(rules)
Expand Down Expand Up @@ -141,13 +161,6 @@ const (
_ = jumpIdxPolicy
)

const (
PolRCNoMatch = 0
PolRCAllow = 1
PolRCDeny = 2
PolRCEpilogueTailCallFailed = 10
)

// writeProgramFooter emits the program exit jump targets.
func (p *Builder) writeProgramFooter() {
// Fall through here if there's no match. Also used when we hit an error or if policy rejects packet.
Expand All @@ -158,7 +171,7 @@ func (p *Builder) writeProgramFooter() {
if p.b.TargetIsUsed("allow") {
p.b.LabelNextInsn("allow")
// Store the policy result in the state for the next program to see.
p.b.MovImm32(R1, 1)
p.b.MovImm32(R1, int32(state.PolicyAllow))
p.b.Store32(R9, R1, stateOffPolResult)
// Execute the tail call.
p.b.Mov64(R1, R6) // First arg is the context.
Expand All @@ -167,7 +180,7 @@ func (p *Builder) writeProgramFooter() {
p.b.Call(HelperTailCall)

// Fall through if tail call fails.
p.b.MovImm32(R1, PolRCEpilogueTailCallFailed)
p.b.MovImm32(R1, state.PolicyTailCallFailed)
p.b.Store32(R9, R1, stateOffPolResult)
p.b.MovImm64(R0, 2 /* TC_ACT_SHOT */)
p.b.Exit()
Expand Down Expand Up @@ -207,27 +220,52 @@ func (p *Builder) setUpIPSetKey(ipsetID uint64, keyOffset, ipOffset, portOffset
p.b.StoreStack32(R1, keyOffset+ipsKeyID+4)
}

func (p *Builder) writeRules(rules [][][]*proto.Rule) {
for polOrProfIdx, polsOrProfs := range rules {
endOfTierLabel := fmt.Sprint("end_of_tier_", polOrProfIdx)

log.Debugf("Start of policies or profiles %d", polOrProfIdx)
for polIdx, pol := range polsOrProfs {
log.Debugf("Start of policy/profile %d", polIdx)
for ruleIdx, rule := range pol {
log.Debugf("Start of rule %d", ruleIdx)
p.writeRule(rule, endOfTierLabel)
log.Debugf("End of rule %d", ruleIdx)
}
log.Debugf("End of policy/profile %d", polIdx)
func (p *Builder) writeRules(rules Rules) {
for idx, tier := range rules.Tiers {
endOfTierLabel := fmt.Sprint("end_of_tier_", idx)

log.Debugf("Start of policies %d", idx)
for polIdx, pol := range tier.Policies {
p.writePolicy(pol, polIdx, endOfTierLabel)
}

// End of polsOrProfs drop rule.
log.Debugf("End of policies/profiles drop")
log.Debugf("End of policies drop")
p.writeRule(&proto.Rule{Action: "deny"}, endOfTierLabel)

p.b.LabelNextInsn(endOfTierLabel)
}

endLabel := "end_of_profiles"

log.Debugf("Start of profiles")
for idx, prof := range rules.Profiles {
p.writeProfile(prof, idx, endLabel)
}

log.Debugf("End of profiles drop")
p.writeRule(&proto.Rule{Action: "deny"}, endLabel)

p.b.LabelNextInsn(endLabel)
}

func (p *Builder) writePolicyRules(policy Policy, idx int, endLabel string) {
for ruleIdx, rule := range policy.Rules {
log.Debugf("Start of rule %d", ruleIdx)
p.writeRule(rule, endLabel)
log.Debugf("End of rule %d", ruleIdx)
}
}

func (p *Builder) writePolicy(policy Policy, idx int, endLabel string) {
log.Debugf("Start of policy %q %d", policy.Name, idx)
p.writePolicyRules(policy, idx, endLabel)
log.Debugf("End of policy %q %d", policy.Name, idx)
}

func (p *Builder) writeProfile(profile Profile, idx int, endLabel string) {
log.Debugf("Start of profile %q %d", profile.Name, idx)
p.writePolicyRules(profile, idx, endLabel)
log.Debugf("End of profile %q %d", profile.Name, idx)
}

type matchLeg string
Expand Down
54 changes: 30 additions & 24 deletions bpf/polprog/pol_prog_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,36 @@ func TestPolicySanityCheck(t *testing.T) {
return id
}
pg := NewBuilder(alloc, 1, 2, 3)
insns, err := pg.Instructions([][][]*proto.Rule{{{{
Action: "Allow",
IpVersion: 4,
Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 6}},
SrcNet: []string{"10.0.0.0/8"},
SrcPorts: []*proto.PortRange{{First: 80, Last: 81}, {First: 8080, Last: 8081}},
SrcNamedPortIpSetIds: []string{setID("n:abcdef1234567890")},
DstNet: []string{"11.0.0.0/8"},
DstPorts: []*proto.PortRange{{First: 3000, Last: 3001}},
DstNamedPortIpSetIds: []string{setID("n:foo1234567890")},
Icmp: &proto.Rule_IcmpTypeCode{IcmpTypeCode: &proto.IcmpTypeAndCode{Type: 10, Code: 12}},
SrcIpSetIds: []string{setID("s:sbcdef1234567890")},
DstIpSetIds: []string{setID("s:dbcdef1234567890")},
NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "UDP"}},
NotSrcNet: []string{"12.0.0.0/8"},
NotSrcPorts: []*proto.PortRange{{First: 5000, Last: 5000}},
NotDstNet: []string{"13.0.0.0/8"},
NotDstPorts: []*proto.PortRange{{First: 4000, Last: 4000}},
NotIcmp: &proto.Rule_NotIcmpTypeCode{NotIcmpTypeCode: &proto.IcmpTypeAndCode{Type: 10, Code: 12}},
NotSrcIpSetIds: []string{setID("s:abcdef1234567890")},
NotDstIpSetIds: []string{setID("s:abcdef123456789l")},
NotSrcNamedPortIpSetIds: []string{setID("n:0bcdef1234567890")},
NotDstNamedPortIpSetIds: []string{setID("n:0bcdef1234567890")},
}}}})
insns, err := pg.Instructions(Rules{
Tiers: []Tier{{
Policies: []Policy{{
Rules: []*proto.Rule{{
Action: "Allow",
IpVersion: 4,
Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Number{Number: 6}},
SrcNet: []string{"10.0.0.0/8"},
SrcPorts: []*proto.PortRange{{First: 80, Last: 81}, {First: 8080, Last: 8081}},
SrcNamedPortIpSetIds: []string{setID("n:abcdef1234567890")},
DstNet: []string{"11.0.0.0/8"},
DstPorts: []*proto.PortRange{{First: 3000, Last: 3001}},
DstNamedPortIpSetIds: []string{setID("n:foo1234567890")},
Icmp: &proto.Rule_IcmpTypeCode{IcmpTypeCode: &proto.IcmpTypeAndCode{Type: 10, Code: 12}},
SrcIpSetIds: []string{setID("s:sbcdef1234567890")},
DstIpSetIds: []string{setID("s:dbcdef1234567890")},
NotProtocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "UDP"}},
NotSrcNet: []string{"12.0.0.0/8"},
NotSrcPorts: []*proto.PortRange{{First: 5000, Last: 5000}},
NotDstNet: []string{"13.0.0.0/8"},
NotDstPorts: []*proto.PortRange{{First: 4000, Last: 4000}},
NotIcmp: &proto.Rule_NotIcmpTypeCode{NotIcmpTypeCode: &proto.IcmpTypeAndCode{Type: 10, Code: 12}},
NotSrcIpSetIds: []string{setID("s:abcdef1234567890")},
NotDstIpSetIds: []string{setID("s:abcdef123456789l")},
NotSrcNamedPortIpSetIds: []string{setID("n:0bcdef1234567890")},
NotDstNamedPortIpSetIds: []string{setID("n:0bcdef1234567890")},
}},
}},
}},
})

Expect(err).NotTo(HaveOccurred())
for i, in := range insns {
Expand Down
11 changes: 10 additions & 1 deletion bpf/state/map.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ import (
"github.com/projectcalico/felix/bpf"
)

type PolicyResult int32

const (
PolicyNoMatch PolicyResult = iota
PolicyAllow
PolicyDeny
PolicyTailCallFailed = 10
)

// struct cali_tc_state {
// __be32 ip_src;4
// __be32 ip_dst;8
Expand All @@ -42,7 +51,7 @@ type State struct {
DstAddr uint32
PostNATDstAddr uint32
NATTunSrcAddr uint32
PolicyRC int32
PolicyRC PolicyResult
SrcPort uint16
DstPort uint16
PostNATDstPort uint16
Expand Down
22 changes: 15 additions & 7 deletions bpf/ut/bpf_prog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,18 @@ const (
)

var (
rulesDefaultAllow = [][][]*proto.Rule{{{{Action: "Allow"}}}}
node1ip = net.IPv4(10, 10, 0, 1).To4()
node1ip2 = net.IPv4(10, 10, 2, 1).To4()
node2ip = net.IPv4(10, 10, 0, 2).To4()
rulesDefaultAllow = polprog.Rules{
Tiers: []polprog.Tier{{
Name: "base tier",
Policies: []polprog.Policy{{
Name: "allow all",
Rules: []*proto.Rule{{Action: "Allow"}},
}},
}},
}
node1ip = net.IPv4(10, 10, 0, 1).To4()
node1ip2 = net.IPv4(10, 10, 2, 1).To4()
node2ip = net.IPv4(10, 10, 0, 2).To4()

node1CIDR = net.IPNet{
IP: node1ip,
Expand Down Expand Up @@ -114,7 +122,7 @@ var retvalToStr = map[int]string{
}

func TestCompileTemplateRun(t *testing.T) {
runBpfTest(t, "calico_to_workload_ep", nil, func(bpfrun bpfProgRunFn) {
runBpfTest(t, "calico_to_workload_ep", polprog.Rules{}, func(bpfrun bpfProgRunFn) {
_, _, _, _, pktBytes, err := testPacketUDPDefault()
Expect(err).NotTo(HaveOccurred())

Expand All @@ -140,7 +148,7 @@ type testLogger interface {
Logf(format string, args ...interface{})
}

func setupAndRun(logger testLogger, loglevel string, section string, rules [][][]*proto.Rule, runFn func(progName string)) {
func setupAndRun(logger testLogger, loglevel string, section string, rules polprog.Rules, runFn func(progName string)) {
tempDir, err := ioutil.TempDir("", "calico-bpf-")
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(tempDir)
Expand Down Expand Up @@ -214,7 +222,7 @@ func setupAndRun(logger testLogger, loglevel string, section string, rules [][][
}

// runBpfTest runs a specific section of the entire bpf program in isolation
func runBpfTest(t *testing.T, section string, rules [][][]*proto.Rule, testFn func(bpfProgRunFn)) {
func runBpfTest(t *testing.T, section string, rules polprog.Rules, testFn func(bpfProgRunFn)) {
RegisterTestingT(t)
setupAndRun(t, "debug", section, rules, func(progName string) {
t.Run(section, func(_ *testing.T) {
Expand Down
17 changes: 13 additions & 4 deletions bpf/ut/icmp_related_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,24 @@ import (
. "github.com/onsi/gomega"

"github.com/projectcalico/felix/bpf/nat"
"github.com/projectcalico/felix/bpf/polprog"
"github.com/projectcalico/felix/bpf/routes"
"github.com/projectcalico/felix/ip"
"github.com/projectcalico/felix/proto"
)

var rulesAllowUDP = [][][]*proto.Rule{{{{
Action: "Allow",
Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "udp"}},
}}}}
var rulesAllowUDP = polprog.Rules{
Tiers: []polprog.Tier{{
Name: "base tier",
Policies: []polprog.Policy{{
Name: "allow all udp",
Rules: []*proto.Rule{{
Action: "Allow",
Protocol: &proto.Protocol{NumberOrName: &proto.Protocol_Name{Name: "udp"}},
}},
}},
}},
}

func TestICMPRelatedPlain(t *testing.T) {
RegisterTestingT(t)
Expand Down
Loading

0 comments on commit 94ca599

Please sign in to comment.