Allow fetching rules and counters via bfcli

[ryantimwilson]

bpfilter daemon keeps a list of active rules and it also writes counters to BPF maps. Sure, we can access the rules/counters via bpftool - counter maps are named bf_cmap_XXX - but it is more convenient to use an API.

$ sudo bpftool map dump name bf_cmap_000201
[{
        "key": 0,
        "value": {
            "packets": 71220,
            "bytes": 112981553
        }
    },{
        "key": 1,
        "value": {
            "packets": 71288,
            "bytes": 112993982
        }
    }

To do this, we need to implement the BF_REQ_GET_RULES and BF_REQ_GET_RULES APIs in bpfilter backend and fetch the results via bfcli.

bfcli API could look like (where chain/rule are optional):

$ bfcli get-rules <chain> <rule>
$ bfcli get-counters <chain> <rule>

Note bfcli would need to introduce subcommands as bfcli is only used for setting rules now.

[qdeslandes]

Some thoughts on this:

• BF_REQ_GET_RULES API is as old as this repository, the daemon changed a lot since then, you might have to modify that logic a bit.
• bfcli get-rules would be a problem on its own: there is no logic to convert the ruleset back to the format used by bfcli. Maybe it would be worth to piggy-back on bf_xxx_dump() logic? You can print almost anything with bf_xxx_dump(), except for a matcher's payload (but that's easily solvable).
• bfcli get-counters: nft prints the counters with the rule's definition, that could be a way to do it.

[ryantimwilson]

Tommy Unger is working on adding get-counters functionality as part of his ramp-up!

[tommy-u]

I have a few noob questions here. I'm currently working on get-counters because we agreed that get-rules will require further conversation.

1. Could it be that we actually want <chain> and <rule> to be required arguments to get-counters? If <chain> and <rule> are optional, what is the expected functionality when one, the other, or both are omitted? Perhaps omitting both shows counters from all chains and all rules. Omitting <rule> shows all counters from all rules in the given <chain>. Omitting <chain> shows all counters corresponding to matching rules across all chains (is this useful?).
2. Is there an existing bfcli request this get-counters request can extend/piggyback on? After looking it seems like the answer may be "no;" the closest analog I could find was bf_ipt_get_info(). Even if that were usable, we would want to implement a new function using BF_FRONT_CLI, right?
3. Do I have it right that implementing get-counters as a sub-command is best done by modifying the bfcli parser?

Hope these questions even make sense. I'm only beginning to understand this (very cool) system!

[tommy-u]

I think I've more or less resolved 2 above. I'm currently working on reading a value out of an existing map. I see bf_map_new() generating file paths for maps like /sys/fs/bpf/bf_cmap_030200. I expected to find a psudo-file there that existed for the lifetime of the bpfilter daemon (after receiving a rule from the CLI), but I don't see any. I assumed this would be the primary handle for interacting with existing maps from userspace (e.g. you'd get a fd using it).

Perhaps it: 1) only lives there transiently, 2) is never actually created, or 3) I need to enable some configuration to show these files.

Currently puzzling on that, will report back if I get to the bottom of it.

[qdeslandes]

Could it be that we actually want and to be required arguments to get-counters? If and are optional, what is the expected functionality when one, the other, or both are omitted? Perhaps omitting both shows counters from all chains and all rules. Omitting shows all counters from all rules in the given . Omitting shows all counters corresponding to matching rules across all chains (is this useful?).

One issue I see with get-counters is that counters only make sense when associated with their rule. I.e. bfcli get-counters $MYCHAIN $MYRULE requires the user to know the chain's name or ID, and the rule's ID, which they can't as get-rules doesn't exist.

It would make more sense to start with a high-level command to fetch the whole ruleset (including the counters), i.e.:

$ bfcli ruleset
chain BF_HOOK_XDP{attach=no,ifindex=2,name=my_xdp_program} policy ACCEPT
    rule
        meta.dport eq 22
        counter 12412 packets 1249120 bytes
        CONTINUE
    rule
        meta.dport not 22
        counter 12 packets 1841 bytes
        CONTINUE

A modified version of bf_chain_dump() (bf_chain_print()?) could be used to print a chain (and a rule) as above, including the counters.

Is there an existing bfcli request this get-counters request can extend/piggyback on? After looking it seems like the answer may be "no;" the closest analog I could find was bf_ipt_get_info().

Following the example above, bf_request_cmd should include BF_REQ_GET_RULESET to retrieve the whole ruleset.

Even if that were usable, we would want to implement a new function using BF_FRONT_CLI, right?

You're right, this new command should be supported specifically for BF_FRONT_CLI.

Do I have it right that implementing get-counters as a sub-command is best done by modifying the bfcli parser?

You would have to modify the arguments' parser, not the Bison parser to support subparser with argp.

Currently, bfcli is only used to define a ruleset and doesn't support subcommands. I would suggest the following plan:

• Move the ruleset definition behind a subcommand, e.g. bfcli ruleset set ..., which supports the current ruleset definition options (i.e. --file and --str)
• Introduce a new command to fetch the ruleset: bfcli ruleset get (with get optional, ideally)

I think I've more or less resolved 2 above. I'm currently working on reading a value out of an existing map.

I see you started working on this, and I'm changing the requirements and expectations on the fly, sorry about that :/

I see bf_map_new() generating file paths for maps like /sys/fs/bpf/bf_cmap_030200. I expected to find a psudo-file there that existed for the lifetime of the bpfilter daemon (after receiving a rule from the CLI), but I don't see any. I assumed this would be the primary handle for interacting with existing maps from userspace (e.g. you'd get a fd using it).

When creating a map, bpfilter will always associate a path to it. The map will be pinned to this path unless the daemon is run in transient mode (--transient). I assume you run the daemon with --transient? That would explain why the path is empty.

Regarding this task, it's better if bfcli goes through the daemon directly, instead of reading from a map, as the layout in the map is only known by the daemon.

[tommy-u]

UPDATE EDIT: I have since found bf_list_marsh(), which I believe makes the former approach easy. Feel free to disregard.

I have a question about the marshaled data with which the daemon responds to the CLI. Is there a preference for returning a list of chains over, say, a cgen (or ctx for that matter)? Only asking because it seems the former will require writing new marshaling/unmarshaling code while it is already done for the latter.

Thanks!

[tommy-u]

I'm not quite there yet, but an issue I see coming for ruleset get has to do with providing the per-rule counters. I believe this information is not held within the chains, so it does not seem there is a direct way to provide these to the CLI. I'm curious if I'm missing something simple here.

If I have that right, It might take extra work to marshal them into the daemon's response. Or perhaps ruleset get can provide the rules and indices for the CLI user to use to access the counters using get-counters.

[qdeslandes]

@tommy-u

UPDATE EDIT: I have since found bf_list_marsh(), which I believe makes the former approach easy. Feel free to disregard.

I have a question about the marshaled data with which the daemon responds to the CLI. Is there a preference for returning a list of chains over, say, a cgen (or ctx for that matter)? Only asking because it seems the former will require writing new marshaling/unmarshaling code while it is already done for the latter.

That's right, it should help to serialise a whole list at once. bf_list_unmarsh() doesn't exist, though!

I'm not quite there yet, but an issue I see coming for ruleset get has to do with providing the per-rule counters. I believe this information is not held within the chains, so it does not seem there is a direct way to provide these to the CLI. I'm curious if I'm missing something simple here.

If I have that right, It might take extra work to marshal them into the daemon's response. Or perhaps ruleset get can provide the rules and indices for the CLI user to use to access the counters using get-counters.

You're right. But that's fine by me, the chains are static, but the counters are dynamic. bfcli ruleset get would return the entire ruleset, but not the counters. That's the first step.

Then, bfcli ruleset get could provide something like --with-counters flag, in which case it would request the counters (e.g. BF_REQ_COUNTERS_GET) before printing the ruleset + the counters (similarly to nftables). The response from the daemon could be serialized from:

main bf_marsh
    bf_marsh of a list of counters for chain #1
        bf_marsh for the counters of rule #1 in chain #1
        [...]
    bf_marsh of a list of counters for chain #2
        [...]