Re: [ovs-dev] [RFC v2 0/9] Introduce local sampling with NXAST_SAMPLE action

2024-06-21 Thread Eelco Chaudron


On 21 Jun 2024, at 10:00, Adrián Moreno wrote:

> 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?

You are right, I did not notice these log messages :( Doing too much stuff in 
parallel...

//Eelco

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


Re: [ovs-dev] [RFC v2 0/9] Introduce local sampling with NXAST_SAMPLE action

2024-06-21 Thread Adrián Moreno
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


Re: [ovs-dev] [RFC v2 0/9] Introduce local sampling with NXAST_SAMPLE action

2024-06-20 Thread Eelco Chaudron


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?

Cheers,

Eelco

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


[ovs-dev] [RFC v2 0/9] Introduce local sampling with NXAST_SAMPLE action

2024-06-05 Thread Adrian Moreno
(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/

Adrian Moreno (9):
  odp-util: Add support OVS_ACTION_ATTR_EMIT_SAMPLE.
  ofproto_dpif: Check for emit_sample support.
  ofproto: Add ofproto-dpif-lsample.
  vswitchd: Add local sampling to vswitchd schema.
  ofproto-dpif-xlate: Use emit_sample for local sample.
  ofproto-dpif-xlate-cache: Add lsample to xcache.
  ofproto-dpif-lsample: Show stats via unixctl.
  tests: Add test-psample testing utility.
  tests: Test local sampling.

 NEWS   |   6 +
 include/linux/automake.mk  |   1 +
 include/linux/openvswitch.h|  25 +++
 include/linux/psample.h|  68 ++
 lib/dpif-netdev.c  |   1 +
 lib/dpif.c |   8 +
 lib/dpif.h |   1 +
 lib/odp-execute.c  |  25 ++-
 lib/odp-util.c | 103 +
 lib/odp-util.h |   3 +
 ofproto/automake.mk|   2 +
 ofproto/ofproto-dpif-ipfix.c   |   1 +
 ofproto/ofproto-dpif-lsample.c | 333 +
 ofproto/ofproto-dpif-lsample.h |  45 
 ofproto/ofproto-dpif-sflow.c   |   1 +
 ofproto/ofproto-dpif-xlate-cache.c |  11 +-
 ofproto/ofproto-dpif-xlate-cache.h |   6 +
 ofproto/ofproto-dpif-xlate.c   | 178 +++
 ofproto/ofproto-dpif-xlate.h   |   3 +-
 ofproto/ofproto-dpif.c |  86 +++-
 ofproto/ofproto-dpif.h |   7 +-
 ofproto/ofproto-provider.h |   9 +
 ofproto/ofproto.c  |  12 ++
 ofproto/ofproto.h  |   8 +
 python/ovs/flow/odp.py |   8 +
 tests/automake.mk  |   3 +-
 tests/odp.at   |  16 ++
 tests/system-common-macros.at  |   4 +
 tests/system-traffic.at| 105 +
 tests/test-psample.c   | 282 
 vswitchd/bridge.c  |  78 ++-
 vswitchd/vswitch.ovsschema |   9 +-
 vswitchd/vswitch.xml   |  39 +++-
 33 files changed, 1425 insertions(+), 62 deletions(-)
 create mode 100644 include/linux/psample.h
 create mode 100644 ofpro