[ovs-dev] [PATCH v15 6/7] ofproto: Introduce API to process sFlow offload packet
Eelco Chaudron
echaudro at redhat.com
Fri Oct 1 12:24:05 UTC 2021
See comments below.
On 15 Sep 2021, at 14:43, Chris Mi wrote:
> Process sFlow offload packet in handler thread if handler id is 0.
>
> Signed-off-by: Chris Mi <cmi at nvidia.com>
> Reviewed-by: Eli Britstein <elibr at nvidia.com>
> ---
> ofproto/ofproto-dpif-upcall.c | 63 +++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 1c9c720f0..4a36a45bb 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"
> @@ -779,6 +780,57 @@ 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;
> + const 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: sFlow upcall is missing its attribute",
> + __func__);
Here we should remove the function in the log message. A developer can easily find it, and it might confuse the end-user.
> + return;
> + }
> +
> + cookie = nl_attr_get(attr->userdata);
Here we need to also check that the length of the attr is at least sizeof(struct user_action_cookie)
> + if (!cookie) {
> + VLOG_WARN_RL(&rl, "%s: user action cookie is missing", __func__);
Remove function name, and make it sflow specific;
“sFlow user action cookie is missing”
> + return;
> + }
> + ofproto = ofproto_dpif_lookup_by_uuid(&cookie->ofproto_uuid);
> + if (!ofproto) {
> + VLOG_WARN_RL(&rl, "%s: sFlow upcall can't find ofproto dpif for UUID "
> + UUID_FMT, __func__, UUID_ARGS(&cookie->ofproto_uuid));
Please remove function name.
> + return;
> + }
> + dpif_sflow = ofproto->sflow;
> + if (!dpif_sflow) {
> + VLOG_WARN_RL(&rl, "%s: sFlow upcall is missing dpif information",
> + __func__);
Please remove function name.
> + 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 = &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. */
> @@ -795,6 +847,17 @@ udpif_upcall_handler(void *arg)
> dpif_recv_wait(udpif->dpif, handler->handler_id);
> 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;
> +
> + dpif_offload_sflow_recv_wait(udpif->dpif);
> + err = dpif_offload_sflow_recv(udpif->dpif, &sflow);
> + if (!err) {
> + process_offload_sflow(udpif, &sflow);
> + }
Here we only read and process a single sflow upcall for each poll block. Should we not do the same as the upcall handling? It's reading max 64 entries before continuation?
> + }
> poll_block();
> }
>
> --
> 2.27.0
More information about the dev
mailing list