Skip to content

Commit

Permalink
Merge pull request #9137 from tomastigera/tomas-bpf-fix-log-filter-idxs
Browse files Browse the repository at this point in the history 
[BPF] fix applying and removing log filters
  • Loading branch information
@tomastigera
tomastigera authored Aug 19, 2024
2 parents 97f526c + 6c7c63e commit f747905
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 39 deletions.
91 changes: 91 additions & 0 deletions felix/bpf/ut/attach_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,97 @@ func TestRepeatedAttach(t *testing.T) {
}
}

func TestLogFilters(t *testing.T) {
RegisterTestingT(t)

bpfmaps, err := bpfmap.CreateBPFMaps(false)
Expect(err).NotTo(HaveOccurred())

commonMaps := bpfmaps.CommonMaps

cfg := linux.Config{
Hostname: "uthost",
BPFLogLevel: "debug",
BPFDataIfacePattern: regexp.MustCompile("^hostep[12]"),
VXLANMTU: 1000,
VXLANPort: 1234,
BPFNodePortDSREnabled: false,
RulesConfig: rules.Config{
EndpointToHostAction: "RETURN",
},
BPFExtToServiceConnmark: 0,
FeatureGates: map[string]string{
"BPFConnectTimeLoadBalancingWorkaround": "enabled",
},
BPFPolicyDebugEnabled: true,
BPFLogFilters: map[string]string{"hostep1": "tcp"},
}

bpfEpMgr, err := linux.NewTestEpMgr(
&cfg,
bpfmaps,
regexp.MustCompile("^workloadep[0123]"),
)
Expect(err).NotTo(HaveOccurred())

host1 := createVethName("hostep1")
defer deleteLink(host1)

workload0 := createVethName("workloadep0")
defer deleteLink(workload0)

t.Run("load filter", func(t *testing.T) {
bpfEpMgr.OnUpdate(linux.NewIfaceStateUpdate("hostep1", ifacemonitor.StateUp, host1.Attrs().Index))
bpfEpMgr.OnUpdate(linux.NewIfaceStateUpdate("workloadep0", ifacemonitor.StateUp, workload0.Attrs().Index))
bpfEpMgr.OnUpdate(linux.NewIfaceAddrsUpdate("hostep1", "1.2.3.4"))
bpfEpMgr.OnUpdate(&proto.HostMetadataUpdate{Hostname: "uthost", Ipv4Addr: "1.2.3.4"})
err = bpfEpMgr.CompleteDeferredWork()
Expect(err).NotTo(HaveOccurred())

ifstateMap := ifstateMapDump(commonMaps.IfStateMap)

Expect(ifstateMap).To(HaveKey(ifstate.NewKey(uint32(host1.Attrs().Index))))
hostep1State := ifstateMap[ifstate.NewKey(uint32(host1.Attrs().Index))]
Expect(hostep1State.TcIngressFilter()).NotTo(Equal(-1))
Expect(hostep1State.TcEgressFilter()).NotTo(Equal(-1))

Expect(ifstateMap).To(HaveKey(ifstate.NewKey(uint32(workload0.Attrs().Index))))
wl0State := ifstateMap[ifstate.NewKey(uint32(workload0.Attrs().Index))]
Expect(wl0State.TcIngressFilter()).To(Equal(-1))
Expect(wl0State.TcEgressFilter()).To(Equal(-1))
})

cfg.BPFLogLevel = "off"

bpfEpMgr, err = linux.NewTestEpMgr(
&cfg,
bpfmaps,
regexp.MustCompile("^workloadep[0123]"),
)
Expect(err).NotTo(HaveOccurred())

t.Run("after restart, load filter", func(t *testing.T) {
bpfEpMgr.OnUpdate(linux.NewIfaceStateUpdate("hostep1", ifacemonitor.StateUp, host1.Attrs().Index))
bpfEpMgr.OnUpdate(linux.NewIfaceStateUpdate("workloadep0", ifacemonitor.StateUp, workload0.Attrs().Index))
bpfEpMgr.OnUpdate(linux.NewIfaceAddrsUpdate("hostep1", "1.2.3.4"))
bpfEpMgr.OnUpdate(&proto.HostMetadataUpdate{Hostname: "uthost", Ipv4Addr: "1.2.3.4"})
err = bpfEpMgr.CompleteDeferredWork()
Expect(err).NotTo(HaveOccurred())

ifstateMap := ifstateMapDump(commonMaps.IfStateMap)

Expect(ifstateMap).To(HaveKey(ifstate.NewKey(uint32(host1.Attrs().Index))))
hostep1State := ifstateMap[ifstate.NewKey(uint32(host1.Attrs().Index))]
Expect(hostep1State.TcIngressFilter()).To(Equal(-1))
Expect(hostep1State.TcEgressFilter()).To(Equal(-1))

Expect(ifstateMap).To(HaveKey(ifstate.NewKey(uint32(workload0.Attrs().Index))))
wl0State := ifstateMap[ifstate.NewKey(uint32(workload0.Attrs().Index))]
Expect(wl0State.TcIngressFilter()).To(Equal(-1))
Expect(wl0State.TcEgressFilter()).To(Equal(-1))
})
}

func ifstateMapDump(m maps.Map) ifstate.MapMem {
ifstateMap := make(ifstate.MapMem)
ifstateMapIter := ifstate.MapMemIter(ifstateMap)
Expand Down
74 changes: 53 additions & 21 deletions felix/dataplane/linux/bpf_ep_mgr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,7 @@ func (m *bpfEndpointManager) reclaimFilterIdx(name string, iface *bpfInterface)
if err := m.jumpMapAlloc.Put(iface.dpState.filterIdx[attachHook], name); err != nil {
log.WithError(err).Errorf("Filter hook %s", attachHook)
}
iface.dpState.filterIdx[attachHook] = -1
}
}

Expand Down Expand Up @@ -984,6 +985,7 @@ func (m *bpfEndpointManager) updateIfaceStateMap(name string, iface *bpfInterfac
if iface.dpState.v6Readiness != ifaceNotReady {
flags |= ifstate.FlgIPv6Ready
}

v := ifstate.NewValue(flags, name,
iface.dpState.v4.policyIdx[hook.XDP],
iface.dpState.v4.policyIdx[hook.Ingress],
Expand Down Expand Up @@ -1725,7 +1727,6 @@ func (m *bpfEndpointManager) doApplyPolicyToDataIface(iface, masterIface string,
)

m.ifacesLock.Lock()
ifaceName := iface
m.withIface(iface, func(iface *bpfInterface) bool {
up = iface.info.ifaceIsUp()
state = iface.dpState
Expand All @@ -1736,9 +1737,6 @@ func (m *bpfEndpointManager) doApplyPolicyToDataIface(iface, masterIface string,
log.WithField("iface", iface).Debug("Ignoring interface that is down")
return state, nil
}
if err := m.dataIfaceStateFillJumps(ifaceName, attachXDPOnly, &state); err != nil {
return state, err
}

hepIface := iface
if !attachXDPOnly {
Expand All @@ -1762,6 +1760,10 @@ func (m *bpfEndpointManager) doApplyPolicyToDataIface(iface, masterIface string,
var xdpAP4, xdpAP6 *xdp.AttachPoint

tcAttachPoint := m.calculateTCAttachPoint(iface)
if err := m.dataIfaceStateFillJumps(tcAttachPoint, attachXDPOnly, &state); err != nil {
return state, err
}

xdpAttachPoint := &xdp.AttachPoint{
AttachPoint: bpf.AttachPoint{
Hook: hook.XDP,
Expand Down Expand Up @@ -2089,72 +2091,93 @@ func (m *bpfEndpointManager) allocJumpIndicesForDataIface(ifaceName string, atta
return nil
}

func (m *bpfEndpointManager) wepStateFillJumps(ifaceName string, state *bpfInterfaceState) error {
func (m *bpfEndpointManager) wepStateFillJumps(ap *tc.AttachPoint, state *bpfInterfaceState) error {
var err error

// Allocate indices for IPv4
if m.v4 != nil {
err = m.allocJumpIndicesForWEP(ifaceName, &state.v4)
err = m.allocJumpIndicesForWEP(ap.IfaceName(), &state.v4)
if err != nil {
return err
}
}

// Allocate indices for IPv6
if m.v6 != nil {
err = m.allocJumpIndicesForWEP(ifaceName, &state.v6)
err = m.allocJumpIndicesForWEP(ap.IfaceName(), &state.v6)
if err != nil {
return err
}
}

if m.bpfLogLevel == "debug" {
if ap.LogLevel == "debug" {
if state.filterIdx[hook.Ingress] == -1 {
state.filterIdx[hook.Ingress], err = m.jumpMapAlloc.Get(ifaceName)
state.filterIdx[hook.Ingress], err = m.jumpMapAlloc.Get(ap.IfaceName())
if err != nil {
return err
}
}
if state.filterIdx[hook.Egress] == -1 {
state.filterIdx[hook.Egress], err = m.jumpMapAlloc.Get(ifaceName)
state.filterIdx[hook.Egress], err = m.jumpMapAlloc.Get(ap.IfaceName())
if err != nil {
return err
}
}
} else {
for _, attachHook := range []hook.Hook{hook.Ingress, hook.Egress} {
if err := m.jumpMapDelete(attachHook, state.filterIdx[attachHook]); err != nil {
log.WithError(err).Warn("Filter program may leak.")
}
if err := m.jumpMapAlloc.Put(state.filterIdx[attachHook], ap.IfaceName()); err != nil {
log.WithError(err).Errorf("Filter hook %s", attachHook)
}
state.filterIdx[attachHook] = -1
}
}

return nil
}

func (m *bpfEndpointManager) dataIfaceStateFillJumps(ifaceName string, attachXDPOnly bool, state *bpfInterfaceState) error {
func (m *bpfEndpointManager) dataIfaceStateFillJumps(ap *tc.AttachPoint, attachXDPOnly bool, state *bpfInterfaceState) error {
var err error

if m.v4 != nil {
err = m.allocJumpIndicesForDataIface(ifaceName, attachXDPOnly, &state.v4)
err = m.allocJumpIndicesForDataIface(ap.IfaceName(), attachXDPOnly, &state.v4)
if err != nil {
return err
}
}

if m.v6 != nil {
err = m.allocJumpIndicesForDataIface(ifaceName, attachXDPOnly, &state.v6)
err = m.allocJumpIndicesForDataIface(ap.IfaceName(), attachXDPOnly, &state.v6)
if err != nil {
return err
}
}

if m.bpfLogLevel == "debug" {
if ap.LogLevel == "debug" {
if state.filterIdx[hook.Ingress] == -1 {
state.filterIdx[hook.Ingress], err = m.jumpMapAlloc.Get(ifaceName)
state.filterIdx[hook.Ingress], err = m.jumpMapAlloc.Get(ap.IfaceName())
if err != nil {
return err
}
}
if state.filterIdx[hook.Egress] == -1 {
state.filterIdx[hook.Egress], err = m.jumpMapAlloc.Get(ifaceName)
state.filterIdx[hook.Egress], err = m.jumpMapAlloc.Get(ap.IfaceName())
if err != nil {
return err
}
}
} else {
for _, attachHook := range []hook.Hook{hook.Ingress, hook.Egress} {
if err := m.jumpMapDelete(attachHook, state.filterIdx[attachHook]); err != nil {
log.WithError(err).Warn("Filter program may leak.")
}
if err := m.jumpMapAlloc.Put(state.filterIdx[attachHook], ap.IfaceName()); err != nil {
log.WithError(err).Errorf("Filter hook %s", attachHook)
}
state.filterIdx[attachHook] = -1
}
}
return nil
}
Expand Down Expand Up @@ -2191,10 +2214,6 @@ func (m *bpfEndpointManager) doApplyPolicy(ifaceName string) (bpfInterfaceState,
return state, nil
}

if err := m.wepStateFillJumps(ifaceName, &state); err != nil {
return state, err
}

// Otherwise, the interface appears to be present but we may or may not have an endpoint from the
// datastore. If we don't have an endpoint then we'll attach a program to block traffic and we'll
// get the jump map ready to insert the policy if the endpoint shows up.
Expand Down Expand Up @@ -2246,6 +2265,10 @@ func (m *bpfEndpointManager) doApplyPolicy(ifaceName string) (bpfInterfaceState,

ap := m.calculateTCAttachPoint(ifaceName)

if err := m.wepStateFillJumps(ap, &state); err != nil {
return state, err
}

if m.v6 != nil {
wg.Add(1)
go func() {
Expand Down Expand Up @@ -2753,29 +2776,38 @@ func (polDirection PolDirection) Inverse() PolDirection {
}

func (m *bpfEndpointManager) apLogFilter(ap *tc.AttachPoint, iface string) (string, string) {
if m.logFilters == nil {
if len(m.logFilters) == 0 {
return m.bpfLogLevel, ""
}

if m.bpfLogLevel != "debug" {
return m.bpfLogLevel, ""
}

exp, ok := m.logFilters[iface]
if !ok {
if ap.Type == tcdefs.EpTypeWorkload {
if exp, ok := m.logFilters["weps"]; ok {
log.WithField("iface", iface).Debugf("Log filter for weps: %s", exp)
return m.bpfLogLevel, exp
}
}
if ap.Type == tcdefs.EpTypeHost {
if exp, ok := m.logFilters["heps"]; ok {
log.WithField("iface", iface).Debugf("Log filter for heps: %s", exp)
return m.bpfLogLevel, exp
}
}
if exp, ok := m.logFilters["all"]; ok {
log.WithField("iface", iface).Debugf("Log filter for all: %s", exp)
return m.bpfLogLevel, exp
}

log.WithField("iface", iface).Debug("No log filter")
return "off", ""
}

log.WithField("iface", iface).Debugf("Log filter: %s", exp)
return m.bpfLogLevel, exp
}

Expand Down
Loading

0 comments on commit f747905

Please sign in to comment.