On 5/10/24 12:45 PM, Adrian Moreno wrote:
On 5/10/24 12:06 PM, Eelco Chaudron wrote:
On 24 Apr 2024, at 21:53, Adrian Moreno wrote:
The new odp sample attributes allow userspace to specify a group_id and
user-defined cookie to be passed down to psample.
Add support for parsing and formatting such action.
Signed-off-by: Adrian Moreno <[email protected]>
Hi Adrian,
See my comments below inline.
//Eelco
---
include/linux/openvswitch.h | 49 +++++++++---
lib/odp-execute.c | 3 +
lib/odp-util.c | 150 ++++++++++++++++++++++++++---------
ofproto/ofproto-dpif-ipfix.c | 2 +
python/ovs/flow/odp.py | 2 +
tests/odp.at | 5 ++
6 files changed, 163 insertions(+), 48 deletions(-)
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index d9fb991ef..3e748405c 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -696,6 +696,7 @@ enum ovs_flow_attr {
#define OVS_UFID_F_OMIT_MASK (1 << 1)
#define OVS_UFID_F_OMIT_ACTIONS (1 << 2)
+#define OVS_PSAMPLE_COOKIE_MAX_SIZE 16
/**
* enum ovs_sample_attr - Attributes for %OVS_ACTION_ATTR_SAMPLE action.
* @OVS_SAMPLE_ATTR_PROBABILITY: 32-bit fraction of packets to sample with
@@ -703,15 +704,27 @@ enum ovs_flow_attr {
* %UINT32_MAX samples all packets and intermediate values sample intermediate
* fractions of packets.
* @OVS_SAMPLE_ATTR_ACTIONS: Set of actions to execute in sampling event.
- * Actions are passed as nested attributes.
+ * Actions are passed as nested attributes. Optional if
+ * OVS_SAMPLE_ATTR_PSAMPLE_GROUP is set.
+ * @OVS_SAMPLE_ATTR_PSAMPLE_GROUP: A 32-bit number to be used as psample group.
+ * Optional if OVS_SAMPLE_ATTR_ACTIONS is set.
+ * @OVS_SAMPLE_ATTR_PSAMPLE_COOKIE: A variable-length binary cookie that, if
+ * provided, will be copied to the psample cookie.
Should we mention any limit on the actual length of the cookie?
Any reason we explicitly call this psample? From an OVS perspective, this
could just be
SAMPLE_ATTR_FORWARD/OFFLOAD/DESTINATION/ENDPOINT/COLLECTOR/TARGET_COOKIE and
_GROUP. Other datapaths, do not have PSAMPLE.
See my response to your comment on the cover-letter for possible name
alternatives.
Thinking about it more, from an OVS perspective we should probably not even
send down a COOKIE, but we should send down an obs_domain_id and obs_point_id,
and then the kernel can move this into a cookie.
I did consider this. However, the opaque cookie fits well with the tc API which
does not have two 32-bit values but an opaque 128-bit cookie. So if the action
is offloaded OVS needs to "encode" the two 32-bit values into the cookie anyways.
The collector itself could be called system/local collector, or something like
that. This way it can use for example psample for kernel and UDST probes for
usespace. Both can pass the group and cookie (obs_domain_id and obs_point_id).
Also, the presence of any of this should not dictate the psample action, we
probably need a specific OVS_SAMPLE_ATTR_SYSTEM_TARGET type nl flag.
It clearer and it might simplify option verification a bit, but isn't a bit
redundant? There is no flag for action execution for instance, the presence of
the attribute is enough.
An alternative would be to have nested attributes, e.g:
enum ovs_sample_attr {
OVS_SAMPLE_ATTR_UNSPEC,
OVS_SAMPLE_ATTR_PROBABILITY, /* u32 number */
OVS_SAMPLE_ATTR_ACTIONS, /* Nested OVS_ACTION_ATTR_* attributes. */
OVS_SAMPLE_ATTR_SYSTEM, /* Nested OVS_SAMPLE_SYSTEM_ATTR_* attributes.
*/
__OVS_SAMPLE_ATTR_MAX,
};
enum ovs_sample_system_attr {
OVS_SAMPLE_SYSTEM_ATTR_OBS_DOMAIN, /* u32 number */
OVS_SAMPLE_SYSTEM_ATTR_OBS_POINT, /* u32 number */
OVS_SAMPLE_SYSTEM_ATTR_COOKIE, /* Nested OVS_ACTION_ATTR_*
__OVS_SSAMPLE_SYSTEM_ATTR_MAX,
};
WDYT?
Another benefit of nested attributes is the easier addition of other
sample-related attributes, I already have a candidate: OVS_SAMPLE_SYSTEM_TRUNC
This will be passed to psample (md->trunc_size) who will truncate the packet
size to the specified value. This can be useful for large packets for which we
are only interested in the headers.
WDYT?
[snip]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
