On Thu, Jun 20, 2024 at 02:20:53PM GMT, Eelco Chaudron wrote:
>
>
> On 5 Jun 2024, at 22:23, Adrian Moreno wrote:
>
> > (Was: Add psample support to NXAST_SAMPLE action)
> >
> > This is the userspace counterpart of the work being done in the kernel
> > [1] which is still not merged (hence the RFC state). There, a new
> > datapath action is added, called "emit_sample". Being a specific
> > action, it promises a more efficient way of making packet samples
> > available to observability applications.
> >
> > From the PoV of ovs-vswitchd, this new action is used to implement
> > "local sampling". Local sampling (or lsample for short) is configured
> > in a similar way as current per-flow IPFIX sampling, i.e: using the
> > Flow_Sample_Collector_Set table and the NXAST_SAMPLE action.
> >
> > However, instead of sending the sample to an external IPFIX collector
> > though the network, the sample is emitted using the new action and
> > made available to locally running sample collector.
> >
> > The specific way emit_sample sends the sample (and the way the local
> > collector shall collect it) is datapath-specific.
> > Currently, currently only the Linux kernel datapath implements it using
> > the psample netlink multicast group.
> >
> > ~~ Configuration ~~
> > Local sampling is configured via a new column in the
> > Flow_Sample_Collector_Set (FSCS) table called "local_sample_group".
> > Configuring this value is orthogonal to also associating the FSCS
> > entry to an entry in the IPFIX table.
> >
> > Once that entry in the OVSDB is configured, NXAST_SAMPLE actions coming
> > from the controller will be translated into the following odp action:
> >
> >    sample(sample={P}%, actions(emit_sample(group={G},cookie={C})))
> >
> > Where:
> >     P: Is the sampling probability from NXAST_SAMPLE
> >     G: Is the group id in the FSCS entry whose "id" matches the one in
> >         the NXAST_SAMPLE.
> >     C: Is a 64bit cookie result of concatenating the obs_domain and
> >     obs_point from the NXAST_SAMPLE, i.e:
> >         "obs_domain << 32 | obs_point"
> > Notes:
> >     - The parent sample action might be omitted if the probability is
> >       100% and there is no IPFIX sampling that requires the use of a
> >       meter.
> >
> > ~~ Internal implementation: dpif-lsample ~~
> > Internally, a new object called "dpif-lsample" is introduced to track
> > the configured local sampling exporters and track statistics based on
> > odp flow stats (using xcache).
> > It exposes the list of configured exporters and their statistics on a
> > new unixctl command called "lsample/show".
> >
> > ~~ Testing ~~
> > The series includes an test utility program than can be executed by
> > running "tests/ovstest test-psample". This utility listens
> > to packets multicasted by the psample module and prints them (also
> > printing the obs_domain and obs_point ids).
> >
> > ~~ HW Offload ~~
> > tc offload is not being introduced in this series as existing sample
> > or userspace actions are not currently offloadable. Also some
> > improvements need to be implemented in tc for it to be feasible.
> >
> > [1]
> > https://patchwork.kernel.org/project/netdevbpf/cover/20240603185647.2310748-1-amore...@redhat.com/
>
> Hi Adrian,
>
> I did not finish reviewing, but I was doing some testing, and I noticed I’m 
> not getting any warning/error when configuring all of this without Datapath 
> support.
>
> Are you planning to add some user feedback when trying to enable this without 
> the kernel other than the “Datapath does not support emit_sample” which the 
> user might not understand relates to this feature?
>

I added the following code in bridge.c when it configures local
sampling:


+    ret = ofproto_set_local_sample(br->ofproto, opts_array, n_opts);
+
+    if (ret == ENOTSUP) {
+        if (n_opts) {
+            VLOG_WARN_RL(&rl,
+                         "bridge %s: ignoring local sampling configuration: "
+                         "not supported by this datapath",
+                         br->name);
+        }
+    } else if (ret) {
+        VLOG_ERR_RL(&rl, "bridge %s: error configuring local sampling: %s",
+                    br->name, ovs_strerror(ret));
+    }

I believe it should warn the user the local sampling configuration will
not be applied.

Is that not working?

> Cheers,
>
> Eelco
>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to