Update used psabpf library for PTF tests (PSA/eBPF) #3261
Conversation
It adds support, compared to earlier version, for: - DirectMeter; - DirectCounter; - set default action; - get table entry (limited to single entry without implementation).
backends/ebpf/tests/ptf/common.py
Outdated
| cmd = "psabpf-ctl table get pipe {} {} ".format(TEST_PIPELINE_ID, table) | ||
| if indirect: | ||
| # TODO: cmd = cmd + "ref " | ||
| self.fail("support for indirect table not supported yet") |
| _, stdout, _ = self.exec_ns_cmd(cmd, "Table get entry failed") | ||
| return json.loads(stdout)[table] | ||
|
|
||
| def table_verify(self, table, keys, action=0, priority=None, data=None, references=None, |
There was a problem hiding this comment.
Some of these functions could use a documentation """
| if priority: | ||
| cmd = cmd + "priority {}".format(priority) | ||
| self.exec_ns_cmd(cmd, "Table {} failed".format(method)) | ||
|
|
||
| def table_add(self, table, keys, action=0, data=None, priority=None, references=None): | ||
| def table_add(self, table, keys, action=0, data=None, priority=None, references=None, |
There was a problem hiding this comment.
have you considered adding type annotation to your python?
I find they help a lot with readability.
So actions are encoded as integers? I wonder whether p4info could be used to use action symbolic names.
There was a problem hiding this comment.
have you considered adding type annotation to your python? I find they help a lot with readability.
At some point of time I had an idea to add that annotations, but I abandoned it. I don't remember why.
EDIT: For now I would keep as is.
So actions are encoded as integers? I wonder whether p4info could be used to use action symbolic names.
Yes, exactly, actions are encoded as integers. We decided that p4info can't be used at the level of psabpf[-cli], so every think what we can comes from BTF side. DirectCounters and DirectMeters are recognized by name, so for actions it should be also possible, but is not implemented yet.
|
@mbudiu-vmw can we merge this? |
It adds support, compared to earlier version, for:
CC: @osinstom @kmateuszssak