On 9/7/2021 4:01 PM, Eelco Chaudron wrote:
On 15 Jul 2021, at 8:01, Chris Mi wrote:
Process sFlow offload packet in handler thread if handler id is 0.
Signed-off-by: Chris Mi <[email protected]>
Reviewed-by: Eli Britstein <[email protected]>
Thanks for making these changes, as this implementation looks way
cleaner than it was before!
---
ofproto/ofproto-dpif-upcall.c | 57
+++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/ofproto/ofproto-dpif-upcall.c
b/ofproto/ofproto-dpif-upcall.c
index ccf97266c..7e934614d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -22,6 +22,7 @@
#include "connmgr.h"
#include "coverage.h"
#include "cmap.h"
+#include "lib/dpif-offload-provider.h"
#include "lib/dpif-provider.h"
#include "dpif.h"
#include "openvswitch/dynamic-string.h"
@@ -742,6 +743,51 @@ udpif_get_n_flows(struct udpif *udpif)
return flow_count;
}
+static void
+process_offload_sflow(struct udpif *udpif, struct
dpif_offload_sflow *sflow)
+{
+ const struct dpif_sflow_attr *attr = sflow->attr;
+ struct user_action_cookie *cookie;
+ struct dpif_sflow *dpif_sflow;
+ struct ofproto_dpif *ofproto;
+ struct upcall upcall;
+ uint32_t iifindex;
+ struct flow flow;
+
+ if (!attr) {
+ VLOG_WARN_RL(&rl, "%s: dpif_sflow_attr is NULL", __func__);
As these are more user log messages, not debug, I would change it to
something like:
|VLOG_WARN_RL(&rl, "sFlow upcall is missing its attribute");|
|Done.|
||
+ return;
+ }
+
+ cookie = attr->userdata;
Just to be safe should we also check if cookie is not NULL + log message?
+ ofproto = ofproto_dpif_lookup_by_uuid(&cookie->ofproto_uuid);
+ if (!ofproto) {
+ VLOG_WARN_RL(&rl, "%s: could not find ofproto", __func__);
Maybe here we could also print the UUID, so it makes it possible to
debug for which tc rule the upcall was.
VLOG_WARN_RL(&rl, "sFlow upcall can't find ofproto dpif for UUID
"UUID_FMT", ...)
Done.
+ return;
+ }
+
+ dpif_sflow = ofproto->sflow;
+ if (!sflow) {
Guess this needs to be dpif_sflow
Indeed, done.
+ VLOG_WARN_RL(&rl, "%s: could not find dpif_sflow", __func__);
Guess there is no find here, so it might be the same as the first LOG
above:
VLOG_WARN_RL(&rl, "sFlow upcall is missing dpif information");
Done.
+ return;
+ }
+
+ memset(&flow, 0, sizeof flow);
+ if (attr->tunnel) {
+ memcpy(&flow.tunnel, attr->tunnel, sizeof flow.tunnel);
+ }
+ iifindex = sflow->iifindex;
+ flow.in_port.odp_port = netdev_ifindex_to_odp_port(iifindex);
+ memset(&upcall, 0, sizeof upcall);
+ upcall.flow = &flow;
+ upcall.cookie = *cookie;
+ upcall.packet = &sflow->packet;
+ upcall.sflow = dpif_sflow;
+ upcall.ufid = &sflow->attr->ufid;
+ upcall.type = SFLOW_UPCALL;
+ process_upcall(udpif, &upcall, NULL, NULL);
+}
+
/* The upcall handler thread tries to read a batch of
UPCALL_MAX_BATCH
* upcalls from dpif, processes the batch and installs
corresponding flows
* in dpif. */
@@ -756,8 +802,19 @@ udpif_upcall_handler(void *arg)
poll_immediate_wake();
} else {
dpif_recv_wait(udpif->dpif, handler->handler_id);
+ dpif_offload_sflow_recv_wait(udpif->dpif);
Guess this only needs to be called from handler 0, same as below.
I changed it like this before. But it didn't work. We can't receive
sampled packets.
I tested this again. Now it works. Not sure what changed :(
Done.
latch_wait(&udpif->exit_latch);
}
+ /* Only handler id 0 thread process sFlow offload packet. */
+ if (handler->handler_id == 0) {
+ struct dpif_offload_sflow sflow;
+ int err;
+
+ err = dpif_offload_sflow_recv(udpif->dpif, &sflow);
+ if (!err) {
+ process_offload_sflow(udpif, &sflow);
+ }
+ }
poll_block();
}
--
2.21.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
