Add --filter-ifname

[jschwinger233]

Set this flag to filter skb from/to the specific interface under the specific netns. If flag --filter-netns is not set, pwru uses current netns.

Signed-off-by: Zhichuan Liang [email protected]

[brb]

@jschwinger233 Maybe let's resolve ifname to ifindex from the user space, and then use ifindex in the kprobe?

[jschwinger233]

@brb That was my idea, but I ended up giving it up because I didn't know how to switch netns using ID.

I know how to switch netns: https://www.weave.works/blog/linux-namespaces-golang-followup, I don't know how to get NsHandle from ID.

I looked up vishvananda/netns, there are 5 functions to get a NsHandle: GetFromDocker, GetFromName, GetFromPath, GetFromPid, GetFromThread, but none accepts a netns ID.

[brb]

I didn't know how to switch netns using ID

A netns ID is just an inode number. Have you tried passing it to https://github.com/vishvananda/netns/blob/master/netns_linux.go#L29, i.e. netns.Set(netns.NsHandle($INODE_NUM))?

[jschwinger233]

I think NsHandle is a wrapper of fd:
https://github.com/vishvananda/netns/blob/16c2fa0b2f57726d0e9e4acd228a63c322954f31/netns_linux.go#L101-L106

The value should be a small integer like 5, 6, 7, instead of something like 4026531834

[brb]

Hmm, you are right. And on Linux getting FD by inode is not possible :-/ Maybe we should change the semantics of filter-netns, and require it to point to a netns path? That would also improve the UX.

[jschwinger233]

I agree that netnsPath improves UX, but netns field in output of pwru --output-meta shows inode. If someone wants to filter a specific netns based on previous pwru output, it would be inconvenient.

(For me, I can use lsns to search pid by inode, but it's still an extra burden for users.)

Speaking of output fields, one more thing is ifindex field in output of pwru --output-meta, I'd love to convert that ifindex to ifname and show something like if=3(eth0), still it's hard to parse ifindex in userspace. We can achieve it in bpf program by fetching skb->dev->name, not sure if we want to do so.

[brb]

I agree that netnsPath improves UX, but netns field in output of pwru --output-meta shows inode. If someone wants to filter a specific netns based on previous pwru output, it would be inconvenient.

My main worry with the current ifname approach is that in the critical path we have a slow operation (string vs int cmp).

WDYT about having --filter-netns (by path) and --filter-netns-by-inode, and then making --filter-netns-by-inode mutual exclusive with --filter-ifname (which we convert to ifindex in the user space)?

[brb]

I'd love to convert that ifindex to ifname and show something like if=3(eth0), still it's hard to parse ifindex in userspace.

It's been on my list since forever 😅 When the prog starts, we could fetch ifindices from all netns, and then build a cache (prone to races, but it's fine) - key=[netns_inode, ifindex] => val=iface_name.

[jschwinger233]

WDYT about having --filter-netns (by path) and --filter-netns-by-inode, and then making --filter-netns-by-inode mutual exclusive with --filter-ifname (which we convert to ifindex in the user space)?

That will do.

Another option is like docker run --network, to adapt different network types, docker run --network accepts:

• bridge as type
• container:<name|id> to attach a container netns
• <network-id>

So how about letting --filter-netns accept multiple forms of arguments:

• path:/proc/1/ns/net
• <inode>

[brb]

So how about letting --filter-netns accept multiple forms of arguments:

SGTM! Just maybe let's make path as a default.

[brb]

Nit: s/proc//proc/

[brb]

@jschwinger233 Very nice! One nit, and then we can

[brb]

@jschwinger233 Could you rebase to resolve the conflict?

[brb]

🚀