On 17 Jun 2021, at 18:27, Kumar Amber wrote:
This patch introduced the auto-validation function which
allows users to compare the batch of packets obtained from
different miniflow implementations against the linear
miniflow extract and return a hitmask.
The autovaidator function can be triggered at runtime using the
following command:
$ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator
Signed-off-by: Kumar Amber <kumar.am...@intel.com>
Co-authored-by: Harry van Haaren <harry.van.haa...@intel.com>
Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com>
---
lib/dpif-netdev-private-extract.c | 141
++++++++++++++++++++++++++++++
lib/dpif-netdev-private-extract.h | 15 ++++
lib/dpif-netdev.c | 2 +-
3 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/lib/dpif-netdev-private-extract.c
b/lib/dpif-netdev-private-extract.c
index fcc56ef26..0741c19f9 100644
--- a/lib/dpif-netdev-private-extract.c
+++ b/lib/dpif-netdev-private-extract.c
@@ -32,6 +32,11 @@ VLOG_DEFINE_THIS_MODULE(dpif_netdev_extract);
/* Implementations of available extract options. */
static struct dpif_miniflow_extract_impl mfex_impls[] = {
+ {
+ .probe = NULL,
+ .extract_func = dpif_miniflow_extract_autovalidator,
+ .name = "autovalidator",
+ },
{
.probe = NULL,
.extract_func = NULL,
@@ -84,3 +89,139 @@ dpif_miniflow_extract_info_get(struct
dpif_miniflow_extract_impl **out_ptr)
*out_ptr = mfex_impls;
return ARRAY_SIZE(mfex_impls);
}
+
+uint32_t
+dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets,
+ struct netdev_flow_key *keys,
+ uint32_t keys_size, odp_port_t
in_port,
+ void *pmd_handle)
+{
+ const size_t cnt = dp_packet_batch_size(packets);
+ uint16_t good_l2_5_ofs[NETDEV_MAX_BURST];
+ uint16_t good_l3_ofs[NETDEV_MAX_BURST];
+ uint16_t good_l4_ofs[NETDEV_MAX_BURST];
+ uint16_t good_l2_pad_size[NETDEV_MAX_BURST];
+ struct dp_packet *packet;
+ struct dp_netdev_pmd_thread *pmd = pmd_handle;
+ struct dpif_miniflow_extract_impl *miniflow_funcs;
+
+ int32_t mfunc_count =
dpif_miniflow_extract_info_get(&miniflow_funcs);
+ if (mfunc_count < 0) {
In theory 0 could not be returned, but just to cover the corner case can
we change this to include zero.
+ pmd->miniflow_extract_opt = NULL;
Guess the above needs to be atomic.
+ VLOG_ERR("failed to get miniflow extract function
implementations\n");
Capital F to be in sync with your other error messages?
+ return 0;
+ }
+ ovs_assert(keys_size >= cnt);
I don’t think we should assert here. Just return an error like above,
so in production, we get notified, and this implementation gets
disabled.
+ struct netdev_flow_key test_keys[NETDEV_MAX_BURST];
+
+ /* Run scalar miniflow_extract to get default result. */
+ DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+ pkt_metadata_init(&packet->md, in_port);
+ miniflow_extract(packet, &keys[i].mf);
+
+ /* Store known good metadata to compare with optimized
metadata. */
+ good_l2_5_ofs[i] = packet->l2_5_ofs;
+ good_l3_ofs[i] = packet->l3_ofs;
+ good_l4_ofs[i] = packet->l4_ofs;
+ good_l2_pad_size[i] = packet->l2_pad_size;
+ }
+
+ /* Iterate through each version of miniflow implementations. */
+ for (int j = MFEX_IMPL_START_IDX; j < ARRAY_SIZE(mfex_impls);
j++) {
+ if (!mfex_impls[j].available) {
+ continue;
+ }
+
+ /* Reset keys and offsets before each implementation. */
+ memset(test_keys, 0, keys_size * sizeof(struct
netdev_flow_key));
+ DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+ dp_packet_reset_offsets(packet);
+ }
+ /* Call optimized miniflow for each batch of packet. */
+ uint32_t hit_mask = mfex_impls[j].extract_func(packets,
test_keys,
+ keys_size, in_port,
pmd_handle);
+
+ /* Do a miniflow compare for bits, blocks and offsets for all
the
+ * classified packets in the hitmask marked by set bits. */
+ while (hit_mask) {
+ /* Index for the set bit. */
+ uint32_t i = __builtin_ctz(hit_mask);
+ /* Set the index in hitmask to Zero. */
+ hit_mask &= (hit_mask - 1);
+
+ uint32_t failed = 0;
+
+ /* Check miniflow bits are equal. */
+ if ((keys[i].mf.map.bits[0] !=
test_keys[i].mf.map.bits[0]) ||
+ (keys[i].mf.map.bits[1] !=
test_keys[i].mf.map.bits[1])) {
+ VLOG_ERR("Good 0x%llx 0x%llx\tTest 0x%llx 0x%llx\n",
+ keys[i].mf.map.bits[0],
keys[i].mf.map.bits[1],
+ test_keys[i].mf.map.bits[0],
+ test_keys[i].mf.map.bits[1]);
+ failed = 1;
+ }
+
+ if (!miniflow_equal(&keys[i].mf, &test_keys[i].mf)) {
+ uint32_t block_cnt = miniflow_n_values(&keys[i].mf);
+ VLOG_ERR("Autovalidation blocks failed for %s pkt
%d",
+ mfex_impls[j].name, i);
+ VLOG_ERR(" Good hexdump:\n");
+ uint64_t *good_block_ptr = (uint64_t *)&keys[i].buf;
+ uint64_t *test_block_ptr = (uint64_t
*)&test_keys[i].buf;
+ for (uint32_t b = 0; b < block_cnt; b++) {
+ VLOG_ERR(" %"PRIx64"\n", good_block_ptr[b]);
+ }
+ VLOG_ERR(" Test hexdump:\n");
+ for (uint32_t b = 0; b < block_cnt; b++) {
+ VLOG_ERR(" %"PRIx64"\n", test_block_ptr[b]);
+ }
+ failed = 1;
+ }
+
+ if ((packets->packets[i]->l2_pad_size !=
good_l2_pad_size[i]) ||
+ (packets->packets[i]->l2_5_ofs !=
good_l2_5_ofs[i]) ||
+ (packets->packets[i]->l3_ofs != good_l3_ofs[i])
||
+ (packets->packets[i]->l4_ofs != good_l4_ofs[i]))
{
+ VLOG_ERR("Autovalidation packet offsets failed for %s
pkt %d",
+ mfex_impls[j].name, i);
+ VLOG_ERR(" Good offsets: l2_pad_size %u, l2_5_ofs :
%u"
+ " l3_ofs %u, l4_ofs %u\n",
+ good_l2_pad_size[i], good_l2_5_ofs[i],
+ good_l3_ofs[i], good_l4_ofs[i]);
+ VLOG_ERR(" Test offsets: l2_pad_size %u, l2_5_ofs :
%u"
+ " l3_ofs %u, l4_ofs %u\n",
+ packets->packets[i]->l2_pad_size,
+ packets->packets[i]->l2_5_ofs,
+ packets->packets[i]->l3_ofs,
+ packets->packets[i]->l4_ofs);
+ failed = 1;
+ }
+
+ if (failed) {
Why stop now!? I think we should run all implementations, as others
might need fixing too!
+ /* Having dumped the debug info, disable
autovalidator. */
+ VLOG_ERR("Autovalidation failed in %s pkt %d,
disabling.\n",
+ mfex_impls[j].name, i);
+ /* Halt OVS here on debug builds. */
+ ovs_assert(0);
+ pmd->miniflow_extract_opt = NULL;
+ break;
+ }
+ }
+ }
+
+ /* preserve packet correctness by storing back the good offsets
in
+ * packets back. */
+ DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+ packet->l2_5_ofs = good_l2_5_ofs[i];
+ packet->l3_ofs = good_l3_ofs[i];
+ packet->l4_ofs = good_l4_ofs[i];
+ packet->l2_pad_size = good_l2_pad_size[i];
+ }
+
+ /* Returning zero implies no packets were hit by autovalidation.
This
+ * simplifies unit-tests as changing
--enable-mfex-default-autovalidator
+ * would pass/fail. By always returning zero, autovalidator is a
little
+ * slower, but we gain consistency in testing.
+ */
+ return 0;
+}
diff --git a/lib/dpif-netdev-private-extract.h
b/lib/dpif-netdev-private-extract.h
index b7b0b2be4..455a7b590 100644
--- a/lib/dpif-netdev-private-extract.h
+++ b/lib/dpif-netdev-private-extract.h
@@ -24,6 +24,11 @@
/* Max size of dpif_miniflow_extract_impl array. */
#define MFEX_IMPLS_MAX_SIZE (16)
+/* Skip the autovalidator study and null when iterating all available
+ * miniflow implementations.
+ */
+#define MFEX_IMPL_START_IDX (1)
+
/* Forward declarations. */
struct dp_packet;
struct miniflow;
@@ -90,5 +95,15 @@ dpif_miniflow_extract_init(void);
int32_t
dpif_miniflow_extract_info_get(struct dpif_miniflow_extract_impl
**out_ptr);
Forgot to comment on this in my previous patch:
/* Function pointer prototype to be implemented in the optimized
miniflow
* extract code.
* returns the hitmask of the processed packets on success.
* returns zero on failure.
*/
typedef uint32_t (*miniflow_extract_func)(struct dp_packet_batch *batch,
struct netdev_flow_key *keys,
uint32_t keys_size,
odp_port_t in_port,
void *pmd_handle);
Maybe we could add some description of what the input parameters mean,
are expected? For example that key_size need to be at least the size of
the batch? If this is the case, do we need it, and can we just assume
keys should be at least batch size long?
+/* Retrieve the hitmask of the batch of pakcets which is obtained by
comparing
+ * different miniflow implementations with linear miniflow extract.
+ * On error, returns a zero.
+ * On success, returns the number of packets in the batch compared.
+ */
+uint32_t
+dpif_miniflow_extract_autovalidator(struct dp_packet_batch *batch,
+ struct netdev_flow_key *keys,
+ uint32_t keys_size, odp_port_t
in_port,
+ void *pmd_handle);
#endif /* DPIF_NETDEV_AVX512_EXTRACT */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 567ebd952..4f4ab2790 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1181,8 +1181,8 @@ dpif_miniflow_extract_impl_set(struct
unixctl_conn *conn, int argc,
struct ds reply = DS_EMPTY_INITIALIZER;
ds_put_format(&reply, "Miniflow implementation set to %s.\n",
mfex_name);
const char *reply_str = ds_cstr(&reply);
- unixctl_command_reply(conn, reply_str);
VLOG_INFO("%s", reply_str);
+ unixctl_command_reply(conn, reply_str);
ds_destroy(&reply);
}
--
2.25.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
