Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[client] Add userspace firewall stats #3104

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[client] Add userspace firewall stats #3104

wants to merge 3 commits into from

Conversation

lixmal
Copy link
Contributor

@lixmal lixmal commented Dec 23, 2024

Describe your changes

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

flow.BytesOut.Add(bytes)
flow.PacketsOut.Add(1)
}
flow.LastSeen = time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

LastSeen is not atomic. Is it a good thing to write under RLock?

m.udpTracker.TrackOutbound(srcIP, dstIP,
uint16(d.udp.SrcPort),
uint16(d.udp.DstPort),
packetData)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you pass the full packetData if you need to the length of it only? Same with ICMP.

@@ -84,6 +90,7 @@ func CreateWithNativeFirewall(iface IFaceMapper, nativeFirewall firewall.Manager

func create(iface IFaceMapper) (*Manager, error) {
disableConntrack, _ := strconv.ParseBool(os.Getenv(EnvDisableConntrack))
enableStats, _ := strconv.ParseBool(os.Getenv(EnvEnableStats))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a warn message would be user-friendly in the case of typo.

@@ -52,6 +55,9 @@ type Manager struct {
udpTracker *conntrack.UDPTracker
icmpTracker *conntrack.ICMPTracker
tcpTracker *conntrack.TCPTracker

statsEnabled bool
Copy link
Contributor

Choose a reason for hiding this comment

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

looks unused variable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants