Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Eelco Chaudron


On 13 May 2024, at 14:50, Adrian Moreno wrote:

> On 5/13/24 2:48 PM, Ilya Maximets wrote:
>> On 5/13/24 13:10, Adrian Moreno wrote:
>>>
>>>
>>> On 5/13/24 12:44 PM, Eelco Chaudron wrote:


 On 13 May 2024, at 9:01, Adrian Moreno wrote:

> On 5/10/24 12:06 PM, Eelco Chaudron wrote:
>> On 24 Apr 2024, at 21:53, Adrian Moreno wrote:
>>
>>> This simple program reads from psample and prints the packets to stdout.
>>> It's useful for quickly collecting sampled packets.
>>
>> See some comments below, did not review the actual sample application in 
>> detail.
>>
>> //Eelco
>>
>>> Signed-off-by: Adrian Moreno 
>>> ---
>>> Documentation/automake.mk   |   1 +
>>> Documentation/conf.py   |   2 +
>>> Documentation/ref/index.rst |   1 +
>>> Documentation/ref/ovs-psample.8.rst |  47 
>>> include/linux/automake.mk   |   1 +
>>> include/linux/psample.h |  64 ++
>>> rhel/openvswitch-fedora.spec.in |   2 +
>>> rhel/openvswitch.spec.in|   1 +
>>> utilities/automake.mk   |   9 +
>>> utilities/ovs-psample.c | 330 
>>> 
>>> 10 files changed, 458 insertions(+)
>>> create mode 100644 Documentation/ref/ovs-psample.8.rst
>>> create mode 100644 include/linux/psample.h
>>> create mode 100644 utilities/ovs-psample.c
>>>
>>> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
>>> index 47d2e336a..c22facfd6 100644
>>> --- a/Documentation/automake.mk
>>> +++ b/Documentation/automake.mk
>>> @@ -165,6 +165,7 @@ RST_MANPAGES = \
>>> ovs-l3ping.8.rst \
>>> ovs-parse-backtrace.8.rst \
>>> ovs-pki.8.rst \
>>> +   ovs-psample.8.rst \
>>> ovs-tcpdump.8.rst \
>>> ovs-tcpundump.1.rst \
>>> ovs-test.8.rst \
>>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>>> index 15785605a..75efed2fc 100644
>>> --- a/Documentation/conf.py
>>> +++ b/Documentation/conf.py
>>> @@ -134,6 +134,8 @@ _man_pages = [
>>>  u'convert "tcpdump -xx" output to hex strings'),
>>> ('ovs-test.8',
>>>  u'Check Linux drivers for performance, vlan and L3 tunneling 
>>> problems'),
>>> +('ovs-psample.8',
>>> + u'Print packets sampled by psample'),
>>> ('ovs-vlan-test.8',
>>>  u'Check Linux drivers for problems with vlan traffic'),
>>> ('ovsdb-server.7',
>>> diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
>>> index 03ada932f..d1076435a 100644
>>> --- a/Documentation/ref/index.rst
>>> +++ b/Documentation/ref/index.rst
>>> @@ -46,6 +46,7 @@ time:
>>>ovs-pki.8
>>>ovs-sim.1
>>>ovs-parse-backtrace.8
>>> +   ovs-psample.8
>>>ovs-tcpdump.8
>>>ovs-tcpundump.1
>>>ovs-test.8
>>> diff --git a/Documentation/ref/ovs-psample.8.rst 
>>> b/Documentation/ref/ovs-psample.8.rst
>>> new file mode 100644
>>> index 0..c849c83d8
>>> --- /dev/null
>>> +++ b/Documentation/ref/ovs-psample.8.rst
>>> @@ -0,0 +1,47 @@
>>> +==
>>> +ovs-sample
>>
>> Interesting, here you call it all ovs-sample here, which is like ;)
>
> Yes, at the begining I called it ovs-sample as I thought to make some 
> generic tool, but since it ended up being very psample-specific I added 
> the "p" (and missed this one).
>
>>
>> You could add options like —local-kernel --local-userspace (--ipfix, 
>> --sflow) and it will eventually work on each datapath.

>
> You mean implementing an IPFIX and sFlow collector?
>
 ?
>> If you want to keep this a separate psample utility, I would not have 
>> ovs in the name, as it's rather general and not OVS specific. Maybe just 
>> psample/psample_mon, as we also have nlmon.
>>
>
> Well, the way the cookie is decoded into observation_domain_id and 
> observation_point_id is ovs-specific.
>
> In fact, for the next iteration of the series I will remove the filtering 
> API since its getting removed from the kernel series as well and add some 
> kind of BPF code that does the filtering. Also I was  considering looking 
> into the OVSDB to ensure that we filter on groups configured in it or 
> else we could wrongly interpet a sampled packet that comes from some 
> other subsystem.

 I would prefer to have this as an ova-sample application, which can be 
 extended with other sample methods as we see fit. So when we added 
 userspace support we can add it here. If we find someone crazy enough to 
 do a simple IPFIX and/or sFlow collector it can be added too.

>>

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Adrian Moreno



On 5/13/24 2:48 PM, Ilya Maximets wrote:

On 5/13/24 13:10, Adrian Moreno wrote:



On 5/13/24 12:44 PM, Eelco Chaudron wrote:



On 13 May 2024, at 9:01, Adrian Moreno wrote:


On 5/10/24 12:06 PM, Eelco Chaudron wrote:

On 24 Apr 2024, at 21:53, Adrian Moreno wrote:


This simple program reads from psample and prints the packets to stdout.
It's useful for quickly collecting sampled packets.


See some comments below, did not review the actual sample application in detail.

//Eelco


Signed-off-by: Adrian Moreno 
---
Documentation/automake.mk   |   1 +
Documentation/conf.py   |   2 +
Documentation/ref/index.rst |   1 +
Documentation/ref/ovs-psample.8.rst |  47 
include/linux/automake.mk   |   1 +
include/linux/psample.h |  64 ++
rhel/openvswitch-fedora.spec.in |   2 +
rhel/openvswitch.spec.in|   1 +
utilities/automake.mk   |   9 +
utilities/ovs-psample.c | 330 
10 files changed, 458 insertions(+)
create mode 100644 Documentation/ref/ovs-psample.8.rst
create mode 100644 include/linux/psample.h
create mode 100644 utilities/ovs-psample.c

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..c22facfd6 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -165,6 +165,7 @@ RST_MANPAGES = \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
+   ovs-psample.8.rst \
ovs-tcpdump.8.rst \
ovs-tcpundump.1.rst \
ovs-test.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..75efed2fc 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -134,6 +134,8 @@ _man_pages = [
 u'convert "tcpdump -xx" output to hex strings'),
('ovs-test.8',
 u'Check Linux drivers for performance, vlan and L3 tunneling 
problems'),
+('ovs-psample.8',
+ u'Print packets sampled by psample'),
('ovs-vlan-test.8',
 u'Check Linux drivers for problems with vlan traffic'),
('ovsdb-server.7',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..d1076435a 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -46,6 +46,7 @@ time:
   ovs-pki.8
   ovs-sim.1
   ovs-parse-backtrace.8
+   ovs-psample.8
   ovs-tcpdump.8
   ovs-tcpundump.1
   ovs-test.8
diff --git a/Documentation/ref/ovs-psample.8.rst 
b/Documentation/ref/ovs-psample.8.rst
new file mode 100644
index 0..c849c83d8
--- /dev/null
+++ b/Documentation/ref/ovs-psample.8.rst
@@ -0,0 +1,47 @@
+==
+ovs-sample


Interesting, here you call it all ovs-sample here, which is like ;)


Yes, at the begining I called it ovs-sample as I thought to make some generic tool, but 
since it ended up being very psample-specific I added the "p" (and missed this 
one).



You could add options like —local-kernel --local-userspace (--ipfix, --sflow) 
and it will eventually work on each datapath.




You mean implementing an IPFIX and sFlow collector?


?

If you want to keep this a separate psample utility, I would not have ovs in 
the name, as it's rather general and not OVS specific. Maybe just 
psample/psample_mon, as we also have nlmon.



Well, the way the cookie is decoded into observation_domain_id and 
observation_point_id is ovs-specific.

In fact, for the next iteration of the series I will remove the filtering API 
since its getting removed from the kernel series as well and add some kind of 
BPF code that does the filtering. Also I was  considering looking into the 
OVSDB to ensure that we filter on groups configured in it or else we could 
wrongly interpet a sampled packet that comes from some other subsystem.


I would prefer to have this as an ova-sample application, which can be extended 
with other sample methods as we see fit. So when we added userspace support we 
can add it here. If we find someone crazy enough to do a simple IPFIX and/or 
sFlow collector it can be added too.

So my request would be to remove the (p) from ovs-psample, and have a switch to 
select --psample (the only supported (MANDATORY) option for now).



Oh, ok. Now I undestand, thank you. We want to leave room for the userspace
implementation. Will do.


FYI, we do have sflow and netflow basic collectors in tests/test-sflow.c and
tests/test-netflow.c.

And I'm not convinced we should maintain an actual collector for any of these.
It's not OVS'es job as a project.  Like we're not shipping nlmon, we probably
shouldn't ship psample or any other collector.  But we can and should have basic
implementations for testing purposes in tests/test-psample.c for example.




Fair enough, I could move it to tests directory. After all, the original reason 
why I wrote this was testing the series. Is that OK with you as well Eelco?


___

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Ilya Maximets
On 5/13/24 13:10, Adrian Moreno wrote:
> 
> 
> On 5/13/24 12:44 PM, Eelco Chaudron wrote:
>>
>>
>> On 13 May 2024, at 9:01, Adrian Moreno wrote:
>>
>>> On 5/10/24 12:06 PM, Eelco Chaudron wrote:
 On 24 Apr 2024, at 21:53, Adrian Moreno wrote:

> This simple program reads from psample and prints the packets to stdout.
> It's useful for quickly collecting sampled packets.

 See some comments below, did not review the actual sample application in 
 detail.

 //Eelco

> Signed-off-by: Adrian Moreno 
> ---
>Documentation/automake.mk   |   1 +
>Documentation/conf.py   |   2 +
>Documentation/ref/index.rst |   1 +
>Documentation/ref/ovs-psample.8.rst |  47 
>include/linux/automake.mk   |   1 +
>include/linux/psample.h |  64 ++
>rhel/openvswitch-fedora.spec.in |   2 +
>rhel/openvswitch.spec.in|   1 +
>utilities/automake.mk   |   9 +
>utilities/ovs-psample.c | 330 
>10 files changed, 458 insertions(+)
>create mode 100644 Documentation/ref/ovs-psample.8.rst
>create mode 100644 include/linux/psample.h
>create mode 100644 utilities/ovs-psample.c
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 47d2e336a..c22facfd6 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -165,6 +165,7 @@ RST_MANPAGES = \
>   ovs-l3ping.8.rst \
>   ovs-parse-backtrace.8.rst \
>   ovs-pki.8.rst \
> + ovs-psample.8.rst \
>   ovs-tcpdump.8.rst \
>   ovs-tcpundump.1.rst \
>   ovs-test.8.rst \
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 15785605a..75efed2fc 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -134,6 +134,8 @@ _man_pages = [
> u'convert "tcpdump -xx" output to hex strings'),
>('ovs-test.8',
> u'Check Linux drivers for performance, vlan and L3 tunneling 
> problems'),
> +('ovs-psample.8',
> + u'Print packets sampled by psample'),
>('ovs-vlan-test.8',
> u'Check Linux drivers for problems with vlan traffic'),
>('ovsdb-server.7',
> diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
> index 03ada932f..d1076435a 100644
> --- a/Documentation/ref/index.rst
> +++ b/Documentation/ref/index.rst
> @@ -46,6 +46,7 @@ time:
>   ovs-pki.8
>   ovs-sim.1
>   ovs-parse-backtrace.8
> +   ovs-psample.8
>   ovs-tcpdump.8
>   ovs-tcpundump.1
>   ovs-test.8
> diff --git a/Documentation/ref/ovs-psample.8.rst 
> b/Documentation/ref/ovs-psample.8.rst
> new file mode 100644
> index 0..c849c83d8
> --- /dev/null
> +++ b/Documentation/ref/ovs-psample.8.rst
> @@ -0,0 +1,47 @@
> +==
> +ovs-sample

 Interesting, here you call it all ovs-sample here, which is like ;)
>>>
>>> Yes, at the begining I called it ovs-sample as I thought to make some 
>>> generic tool, but since it ended up being very psample-specific I added the 
>>> "p" (and missed this one).
>>>

 You could add options like —local-kernel --local-userspace (--ipfix, 
 --sflow) and it will eventually work on each datapath.
>>
>>>
>>> You mean implementing an IPFIX and sFlow collector?
>>>
>> ?
 If you want to keep this a separate psample utility, I would not have ovs 
 in the name, as it's rather general and not OVS specific. Maybe just 
 psample/psample_mon, as we also have nlmon.

>>>
>>> Well, the way the cookie is decoded into observation_domain_id and 
>>> observation_point_id is ovs-specific.
>>>
>>> In fact, for the next iteration of the series I will remove the filtering 
>>> API since its getting removed from the kernel series as well and add some 
>>> kind of BPF code that does the filtering. Also I was  considering looking 
>>> into the OVSDB to ensure that we filter on groups configured in it or else 
>>> we could wrongly interpet a sampled packet that comes from some other 
>>> subsystem.
>>
>> I would prefer to have this as an ova-sample application, which can be 
>> extended with other sample methods as we see fit. So when we added userspace 
>> support we can add it here. If we find someone crazy enough to do a simple 
>> IPFIX and/or sFlow collector it can be added too.
>>
>> So my request would be to remove the (p) from ovs-psample, and have a switch 
>> to select --psample (the only supported (MANDATORY) option for now).
>>
> 
> Oh, ok. Now I undestand, thank you. We want to leave room for the userspace 
> implementation. Will do.

FYI, we do have sflow and netflow basic collectors in tests/test-sflow.c and
tests/test-netfl

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Adrian Moreno



On 5/13/24 12:44 PM, Eelco Chaudron wrote:



On 13 May 2024, at 9:01, Adrian Moreno wrote:


On 5/10/24 12:06 PM, Eelco Chaudron wrote:

On 24 Apr 2024, at 21:53, Adrian Moreno wrote:


This simple program reads from psample and prints the packets to stdout.
It's useful for quickly collecting sampled packets.


See some comments below, did not review the actual sample application in detail.

//Eelco


Signed-off-by: Adrian Moreno 
---
   Documentation/automake.mk   |   1 +
   Documentation/conf.py   |   2 +
   Documentation/ref/index.rst |   1 +
   Documentation/ref/ovs-psample.8.rst |  47 
   include/linux/automake.mk   |   1 +
   include/linux/psample.h |  64 ++
   rhel/openvswitch-fedora.spec.in |   2 +
   rhel/openvswitch.spec.in|   1 +
   utilities/automake.mk   |   9 +
   utilities/ovs-psample.c | 330 
   10 files changed, 458 insertions(+)
   create mode 100644 Documentation/ref/ovs-psample.8.rst
   create mode 100644 include/linux/psample.h
   create mode 100644 utilities/ovs-psample.c

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..c22facfd6 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -165,6 +165,7 @@ RST_MANPAGES = \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
+   ovs-psample.8.rst \
ovs-tcpdump.8.rst \
ovs-tcpundump.1.rst \
ovs-test.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..75efed2fc 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -134,6 +134,8 @@ _man_pages = [
u'convert "tcpdump -xx" output to hex strings'),
   ('ovs-test.8',
u'Check Linux drivers for performance, vlan and L3 tunneling problems'),
+('ovs-psample.8',
+ u'Print packets sampled by psample'),
   ('ovs-vlan-test.8',
u'Check Linux drivers for problems with vlan traffic'),
   ('ovsdb-server.7',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..d1076435a 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -46,6 +46,7 @@ time:
  ovs-pki.8
  ovs-sim.1
  ovs-parse-backtrace.8
+   ovs-psample.8
  ovs-tcpdump.8
  ovs-tcpundump.1
  ovs-test.8
diff --git a/Documentation/ref/ovs-psample.8.rst 
b/Documentation/ref/ovs-psample.8.rst
new file mode 100644
index 0..c849c83d8
--- /dev/null
+++ b/Documentation/ref/ovs-psample.8.rst
@@ -0,0 +1,47 @@
+==
+ovs-sample


Interesting, here you call it all ovs-sample here, which is like ;)


Yes, at the begining I called it ovs-sample as I thought to make some generic tool, but 
since it ended up being very psample-specific I added the "p" (and missed this 
one).



You could add options like —local-kernel --local-userspace (--ipfix, --sflow) 
and it will eventually work on each datapath.




You mean implementing an IPFIX and sFlow collector?


?

If you want to keep this a separate psample utility, I would not have ovs in 
the name, as it's rather general and not OVS specific. Maybe just 
psample/psample_mon, as we also have nlmon.



Well, the way the cookie is decoded into observation_domain_id and 
observation_point_id is ovs-specific.

In fact, for the next iteration of the series I will remove the filtering API 
since its getting removed from the kernel series as well and add some kind of 
BPF code that does the filtering. Also I was  considering looking into the 
OVSDB to ensure that we filter on groups configured in it or else we could 
wrongly interpet a sampled packet that comes from some other subsystem.


I would prefer to have this as an ova-sample application, which can be extended 
with other sample methods as we see fit. So when we added userspace support we 
can add it here. If we find someone crazy enough to do a simple IPFIX and/or 
sFlow collector it can be added too.

So my request would be to remove the (p) from ovs-psample, and have a switch to 
select --psample (the only supported (MANDATORY) option for now).



Oh, ok. Now I undestand, thank you. We want to leave room for the userspace 
implementation. Will do.




+==
+
+Synopsis
+
+
+``ovs-sample``
+[``--group=`` | ``-g`` ]
+
+``ovs-sample --help``
+
+``ovs-sample --version``
+
+Description
+===
+
+Open vSwitch per-flow sampling can be configured to emit the samples
+through the ``psample`` netlink multicast group.
+
+Such sampled traffic contains, apart from the packet, some metadata that
+gives further information about the packet sample. More specifically, OVS
+inserts the ``observation_domain_id`` and the ``observation_point_id`` that
+where provided in the sample action (see ``ovs-actions(7)``).
+
+the ``ovs-sample`` program provides a simple way of joining the psample
+multicast group and printing the sampled packets.
+
+
+Options
+==

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Eelco Chaudron


On 13 May 2024, at 9:01, Adrian Moreno wrote:

> On 5/10/24 12:06 PM, Eelco Chaudron wrote:
>> On 24 Apr 2024, at 21:53, Adrian Moreno wrote:
>>
>>> This simple program reads from psample and prints the packets to stdout.
>>> It's useful for quickly collecting sampled packets.
>>
>> See some comments below, did not review the actual sample application in 
>> detail.
>>
>> //Eelco
>>
>>> Signed-off-by: Adrian Moreno 
>>> ---
>>>   Documentation/automake.mk   |   1 +
>>>   Documentation/conf.py   |   2 +
>>>   Documentation/ref/index.rst |   1 +
>>>   Documentation/ref/ovs-psample.8.rst |  47 
>>>   include/linux/automake.mk   |   1 +
>>>   include/linux/psample.h |  64 ++
>>>   rhel/openvswitch-fedora.spec.in |   2 +
>>>   rhel/openvswitch.spec.in|   1 +
>>>   utilities/automake.mk   |   9 +
>>>   utilities/ovs-psample.c | 330 
>>>   10 files changed, 458 insertions(+)
>>>   create mode 100644 Documentation/ref/ovs-psample.8.rst
>>>   create mode 100644 include/linux/psample.h
>>>   create mode 100644 utilities/ovs-psample.c
>>>
>>> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
>>> index 47d2e336a..c22facfd6 100644
>>> --- a/Documentation/automake.mk
>>> +++ b/Documentation/automake.mk
>>> @@ -165,6 +165,7 @@ RST_MANPAGES = \
>>> ovs-l3ping.8.rst \
>>> ovs-parse-backtrace.8.rst \
>>> ovs-pki.8.rst \
>>> +   ovs-psample.8.rst \
>>> ovs-tcpdump.8.rst \
>>> ovs-tcpundump.1.rst \
>>> ovs-test.8.rst \
>>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>>> index 15785605a..75efed2fc 100644
>>> --- a/Documentation/conf.py
>>> +++ b/Documentation/conf.py
>>> @@ -134,6 +134,8 @@ _man_pages = [
>>>u'convert "tcpdump -xx" output to hex strings'),
>>>   ('ovs-test.8',
>>>u'Check Linux drivers for performance, vlan and L3 tunneling 
>>> problems'),
>>> +('ovs-psample.8',
>>> + u'Print packets sampled by psample'),
>>>   ('ovs-vlan-test.8',
>>>u'Check Linux drivers for problems with vlan traffic'),
>>>   ('ovsdb-server.7',
>>> diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
>>> index 03ada932f..d1076435a 100644
>>> --- a/Documentation/ref/index.rst
>>> +++ b/Documentation/ref/index.rst
>>> @@ -46,6 +46,7 @@ time:
>>>  ovs-pki.8
>>>  ovs-sim.1
>>>  ovs-parse-backtrace.8
>>> +   ovs-psample.8
>>>  ovs-tcpdump.8
>>>  ovs-tcpundump.1
>>>  ovs-test.8
>>> diff --git a/Documentation/ref/ovs-psample.8.rst 
>>> b/Documentation/ref/ovs-psample.8.rst
>>> new file mode 100644
>>> index 0..c849c83d8
>>> --- /dev/null
>>> +++ b/Documentation/ref/ovs-psample.8.rst
>>> @@ -0,0 +1,47 @@
>>> +==
>>> +ovs-sample
>>
>> Interesting, here you call it all ovs-sample here, which is like ;)
>
> Yes, at the begining I called it ovs-sample as I thought to make some generic 
> tool, but since it ended up being very psample-specific I added the "p" (and 
> missed this one).
>
>>
>> You could add options like —local-kernel --local-userspace (--ipfix, 
>> --sflow) and it will eventually work on each datapath.

>
> You mean implementing an IPFIX and sFlow collector?
>
?
>> If you want to keep this a separate psample utility, I would not have ovs in 
>> the name, as it's rather general and not OVS specific. Maybe just 
>> psample/psample_mon, as we also have nlmon.
>>
>
> Well, the way the cookie is decoded into observation_domain_id and 
> observation_point_id is ovs-specific.
>
> In fact, for the next iteration of the series I will remove the filtering API 
> since its getting removed from the kernel series as well and add some kind of 
> BPF code that does the filtering. Also I was  considering looking into the 
> OVSDB to ensure that we filter on groups configured in it or else we could 
> wrongly interpet a sampled packet that comes from some other subsystem.

I would prefer to have this as an ova-sample application, which can be extended 
with other sample methods as we see fit. So when we added userspace support we 
can add it here. If we find someone crazy enough to do a simple IPFIX and/or 
sFlow collector it can be added too.

So my request would be to remove the (p) from ovs-psample, and have a switch to 
select --psample (the only supported (MANDATORY) option for now).

>>> +==
>>> +
>>> +Synopsis
>>> +
>>> +
>>> +``ovs-sample``
>>> +[``--group=`` | ``-g`` ]
>>> +
>>> +``ovs-sample --help``
>>> +
>>> +``ovs-sample --version``
>>> +
>>> +Description
>>> +===
>>> +
>>> +Open vSwitch per-flow sampling can be configured to emit the samples
>>> +through the ``psample`` netlink multicast group.
>>> +
>>> +Such sampled traffic contains, apart from the packet, some metadata that
>>> +gives further information about the packet sample. More specifically, OVS
>>> +inserts the ``observation_domain_id`` and the ``observation_point_id`` that
>>> +

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-13 Thread Adrian Moreno



On 5/10/24 12:06 PM, Eelco Chaudron wrote:

On 24 Apr 2024, at 21:53, Adrian Moreno wrote:


This simple program reads from psample and prints the packets to stdout.
It's useful for quickly collecting sampled packets.


See some comments below, did not review the actual sample application in detail.

//Eelco


Signed-off-by: Adrian Moreno 
---
  Documentation/automake.mk   |   1 +
  Documentation/conf.py   |   2 +
  Documentation/ref/index.rst |   1 +
  Documentation/ref/ovs-psample.8.rst |  47 
  include/linux/automake.mk   |   1 +
  include/linux/psample.h |  64 ++
  rhel/openvswitch-fedora.spec.in |   2 +
  rhel/openvswitch.spec.in|   1 +
  utilities/automake.mk   |   9 +
  utilities/ovs-psample.c | 330 
  10 files changed, 458 insertions(+)
  create mode 100644 Documentation/ref/ovs-psample.8.rst
  create mode 100644 include/linux/psample.h
  create mode 100644 utilities/ovs-psample.c

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..c22facfd6 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -165,6 +165,7 @@ RST_MANPAGES = \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
+   ovs-psample.8.rst \
ovs-tcpdump.8.rst \
ovs-tcpundump.1.rst \
ovs-test.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..75efed2fc 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -134,6 +134,8 @@ _man_pages = [
   u'convert "tcpdump -xx" output to hex strings'),
  ('ovs-test.8',
   u'Check Linux drivers for performance, vlan and L3 tunneling problems'),
+('ovs-psample.8',
+ u'Print packets sampled by psample'),
  ('ovs-vlan-test.8',
   u'Check Linux drivers for problems with vlan traffic'),
  ('ovsdb-server.7',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..d1076435a 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -46,6 +46,7 @@ time:
 ovs-pki.8
 ovs-sim.1
 ovs-parse-backtrace.8
+   ovs-psample.8
 ovs-tcpdump.8
 ovs-tcpundump.1
 ovs-test.8
diff --git a/Documentation/ref/ovs-psample.8.rst 
b/Documentation/ref/ovs-psample.8.rst
new file mode 100644
index 0..c849c83d8
--- /dev/null
+++ b/Documentation/ref/ovs-psample.8.rst
@@ -0,0 +1,47 @@
+==
+ovs-sample


Interesting, here you call it all ovs-sample here, which is like ;)


Yes, at the begining I called it ovs-sample as I thought to make some generic 
tool, but since it ended up being very psample-specific I added the "p" (and 
missed this one).




You could add options like —local-kernel --local-userspace (--ipfix, --sflow) 
and it will eventually work on each datapath.



You mean implementing an IPFIX and sFlow collector?



If you want to keep this a separate psample utility, I would not have ovs in 
the name, as it's rather general and not OVS specific. Maybe just 
psample/psample_mon, as we also have nlmon.



Well, the way the cookie is decoded into observation_domain_id and 
observation_point_id is ovs-specific.


In fact, for the next iteration of the series I will remove the filtering API 
since its getting removed from the kernel series as well and add some kind of 
BPF code that does the filtering. Also I was  considering looking into the OVSDB 
to ensure that we filter on groups configured in it or else we could wrongly 
interpet a sampled packet that comes from some other subsystem.




+==
+
+Synopsis
+
+
+``ovs-sample``
+[``--group=`` | ``-g`` ]
+
+``ovs-sample --help``
+
+``ovs-sample --version``
+
+Description
+===
+
+Open vSwitch per-flow sampling can be configured to emit the samples
+through the ``psample`` netlink multicast group.
+
+Such sampled traffic contains, apart from the packet, some metadata that
+gives further information about the packet sample. More specifically, OVS
+inserts the ``observation_domain_id`` and the ``observation_point_id`` that
+where provided in the sample action (see ``ovs-actions(7)``).
+
+the ``ovs-sample`` program provides a simple way of joining the psample
+multicast group and printing the sampled packets.
+
+
+Options
+===
+
+.. option:: ``-g``  or ``--group`` 
+
+  Tells ``ovs-sample`` to filter out samples that don't belong to that group.
+
+  Different ``Flow_Sample_Collector_Set`` entries can be configured with
+  different ``group_id`` values (see ``ovs-vswitchd.conf.db(5)``). This option
+  helps focusing the output on the relevant samples.
+
+.. option:: -h, --help
+
+Prints a brief help message to the console.
+
+.. option:: -V, --version
+
+Prints version information to the console.
diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index cdae5eedc..ac306b53c 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automak

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-05-10 Thread Eelco Chaudron
On 24 Apr 2024, at 21:53, Adrian Moreno wrote:

> This simple program reads from psample and prints the packets to stdout.
> It's useful for quickly collecting sampled packets.

See some comments below, did not review the actual sample application in detail.

//Eelco

> Signed-off-by: Adrian Moreno 
> ---
>  Documentation/automake.mk   |   1 +
>  Documentation/conf.py   |   2 +
>  Documentation/ref/index.rst |   1 +
>  Documentation/ref/ovs-psample.8.rst |  47 
>  include/linux/automake.mk   |   1 +
>  include/linux/psample.h |  64 ++
>  rhel/openvswitch-fedora.spec.in |   2 +
>  rhel/openvswitch.spec.in|   1 +
>  utilities/automake.mk   |   9 +
>  utilities/ovs-psample.c | 330 
>  10 files changed, 458 insertions(+)
>  create mode 100644 Documentation/ref/ovs-psample.8.rst
>  create mode 100644 include/linux/psample.h
>  create mode 100644 utilities/ovs-psample.c
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 47d2e336a..c22facfd6 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -165,6 +165,7 @@ RST_MANPAGES = \
>   ovs-l3ping.8.rst \
>   ovs-parse-backtrace.8.rst \
>   ovs-pki.8.rst \
> + ovs-psample.8.rst \
>   ovs-tcpdump.8.rst \
>   ovs-tcpundump.1.rst \
>   ovs-test.8.rst \
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 15785605a..75efed2fc 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -134,6 +134,8 @@ _man_pages = [
>   u'convert "tcpdump -xx" output to hex strings'),
>  ('ovs-test.8',
>   u'Check Linux drivers for performance, vlan and L3 tunneling problems'),
> +('ovs-psample.8',
> + u'Print packets sampled by psample'),
>  ('ovs-vlan-test.8',
>   u'Check Linux drivers for problems with vlan traffic'),
>  ('ovsdb-server.7',
> diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
> index 03ada932f..d1076435a 100644
> --- a/Documentation/ref/index.rst
> +++ b/Documentation/ref/index.rst
> @@ -46,6 +46,7 @@ time:
> ovs-pki.8
> ovs-sim.1
> ovs-parse-backtrace.8
> +   ovs-psample.8
> ovs-tcpdump.8
> ovs-tcpundump.1
> ovs-test.8
> diff --git a/Documentation/ref/ovs-psample.8.rst 
> b/Documentation/ref/ovs-psample.8.rst
> new file mode 100644
> index 0..c849c83d8
> --- /dev/null
> +++ b/Documentation/ref/ovs-psample.8.rst
> @@ -0,0 +1,47 @@
> +==
> +ovs-sample

Interesting, here you call it all ovs-sample here, which is like ;)

You could add options like —local-kernel --local-userspace (--ipfix, --sflow) 
and it will eventually work on each datapath.

If you want to keep this a separate psample utility, I would not have ovs in 
the name, as it's rather general and not OVS specific. Maybe just 
psample/psample_mon, as we also have nlmon.

> +==
> +
> +Synopsis
> +
> +
> +``ovs-sample``
> +[``--group=`` | ``-g`` ]
> +
> +``ovs-sample --help``
> +
> +``ovs-sample --version``
> +
> +Description
> +===
> +
> +Open vSwitch per-flow sampling can be configured to emit the samples
> +through the ``psample`` netlink multicast group.
> +
> +Such sampled traffic contains, apart from the packet, some metadata that
> +gives further information about the packet sample. More specifically, OVS
> +inserts the ``observation_domain_id`` and the ``observation_point_id`` that
> +where provided in the sample action (see ``ovs-actions(7)``).
> +
> +the ``ovs-sample`` program provides a simple way of joining the psample
> +multicast group and printing the sampled packets.
> +
> +
> +Options
> +===
> +
> +.. option:: ``-g``  or ``--group`` 
> +
> +  Tells ``ovs-sample`` to filter out samples that don't belong to that group.
> +
> +  Different ``Flow_Sample_Collector_Set`` entries can be configured with
> +  different ``group_id`` values (see ``ovs-vswitchd.conf.db(5)``). This 
> option
> +  helps focusing the output on the relevant samples.
> +
> +.. option:: -h, --help
> +
> +Prints a brief help message to the console.
> +
> +.. option:: -V, --version
> +
> +Prints version information to the console.
> diff --git a/include/linux/automake.mk b/include/linux/automake.mk
> index cdae5eedc..ac306b53c 100644
> --- a/include/linux/automake.mk
> +++ b/include/linux/automake.mk
> @@ -3,6 +3,7 @@ noinst_HEADERS += \
>   include/linux/netfilter/nf_conntrack_sctp.h \
>   include/linux/openvswitch.h \
>   include/linux/pkt_cls.h \
> + include/linux/psample.h \
>   include/linux/gen_stats.h \
>   include/linux/tc_act/tc_mpls.h \
>   include/linux/tc_act/tc_pedit.h \
> diff --git a/include/linux/psample.h b/include/linux/psample.h
> new file mode 100644
> index 0..eb642f875
> --- /dev/null
> +++ b/include/linux/psample.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef __LINUX_PSA

Re: [ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-04-25 Thread 0-day Robot
Bleep bloop.  Greetings Adrian Moreno, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#314 FILE: utilities/ovs-psample.c:52:
"  -t, --group=GROUPonly display events from GROUP group_id\n"

ERROR: Inappropriate bracing around statement
#526 FILE: utilities/ovs-psample.c:264:
if (!e)

ERROR: Inappropriate bracing around statement
#528 FILE: utilities/ovs-psample.c:266:
if (e && e->error < 0)

ERROR: Inappropriate bracing around statement
#577 FILE: utilities/ovs-psample.c:315:
if (error)

Lines checked: 596, Warnings: 2, Errors: 3


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [RFC 06/11] utilities: Add ovs-psample.

2024-04-24 Thread Adrian Moreno
This simple program reads from psample and prints the packets to stdout.
It's useful for quickly collecting sampled packets.

Signed-off-by: Adrian Moreno 
---
 Documentation/automake.mk   |   1 +
 Documentation/conf.py   |   2 +
 Documentation/ref/index.rst |   1 +
 Documentation/ref/ovs-psample.8.rst |  47 
 include/linux/automake.mk   |   1 +
 include/linux/psample.h |  64 ++
 rhel/openvswitch-fedora.spec.in |   2 +
 rhel/openvswitch.spec.in|   1 +
 utilities/automake.mk   |   9 +
 utilities/ovs-psample.c | 330 
 10 files changed, 458 insertions(+)
 create mode 100644 Documentation/ref/ovs-psample.8.rst
 create mode 100644 include/linux/psample.h
 create mode 100644 utilities/ovs-psample.c

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..c22facfd6 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -165,6 +165,7 @@ RST_MANPAGES = \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
+   ovs-psample.8.rst \
ovs-tcpdump.8.rst \
ovs-tcpundump.1.rst \
ovs-test.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..75efed2fc 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -134,6 +134,8 @@ _man_pages = [
  u'convert "tcpdump -xx" output to hex strings'),
 ('ovs-test.8',
  u'Check Linux drivers for performance, vlan and L3 tunneling problems'),
+('ovs-psample.8',
+ u'Print packets sampled by psample'),
 ('ovs-vlan-test.8',
  u'Check Linux drivers for problems with vlan traffic'),
 ('ovsdb-server.7',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..d1076435a 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -46,6 +46,7 @@ time:
ovs-pki.8
ovs-sim.1
ovs-parse-backtrace.8
+   ovs-psample.8
ovs-tcpdump.8
ovs-tcpundump.1
ovs-test.8
diff --git a/Documentation/ref/ovs-psample.8.rst 
b/Documentation/ref/ovs-psample.8.rst
new file mode 100644
index 0..c849c83d8
--- /dev/null
+++ b/Documentation/ref/ovs-psample.8.rst
@@ -0,0 +1,47 @@
+==
+ovs-sample
+==
+
+Synopsis
+
+
+``ovs-sample``
+[``--group=`` | ``-g`` ]
+
+``ovs-sample --help``
+
+``ovs-sample --version``
+
+Description
+===
+
+Open vSwitch per-flow sampling can be configured to emit the samples
+through the ``psample`` netlink multicast group.
+
+Such sampled traffic contains, apart from the packet, some metadata that
+gives further information about the packet sample. More specifically, OVS
+inserts the ``observation_domain_id`` and the ``observation_point_id`` that
+where provided in the sample action (see ``ovs-actions(7)``).
+
+the ``ovs-sample`` program provides a simple way of joining the psample
+multicast group and printing the sampled packets.
+
+
+Options
+===
+
+.. option:: ``-g``  or ``--group`` 
+
+  Tells ``ovs-sample`` to filter out samples that don't belong to that group.
+
+  Different ``Flow_Sample_Collector_Set`` entries can be configured with
+  different ``group_id`` values (see ``ovs-vswitchd.conf.db(5)``). This option
+  helps focusing the output on the relevant samples.
+
+.. option:: -h, --help
+
+Prints a brief help message to the console.
+
+.. option:: -V, --version
+
+Prints version information to the console.
diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index cdae5eedc..ac306b53c 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automake.mk
@@ -3,6 +3,7 @@ noinst_HEADERS += \
include/linux/netfilter/nf_conntrack_sctp.h \
include/linux/openvswitch.h \
include/linux/pkt_cls.h \
+   include/linux/psample.h \
include/linux/gen_stats.h \
include/linux/tc_act/tc_mpls.h \
include/linux/tc_act/tc_pedit.h \
diff --git a/include/linux/psample.h b/include/linux/psample.h
new file mode 100644
index 0..eb642f875
--- /dev/null
+++ b/include/linux/psample.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __LINUX_PSAMPLE_H
+#define __LINUX_PSAMPLE_H
+
+enum {
+   PSAMPLE_ATTR_IIFINDEX,
+   PSAMPLE_ATTR_OIFINDEX,
+   PSAMPLE_ATTR_ORIGSIZE,
+   PSAMPLE_ATTR_SAMPLE_GROUP,
+   PSAMPLE_ATTR_GROUP_SEQ,
+   PSAMPLE_ATTR_SAMPLE_RATE,
+   PSAMPLE_ATTR_DATA,
+   PSAMPLE_ATTR_GROUP_REFCOUNT,
+   PSAMPLE_ATTR_TUNNEL,
+
+   PSAMPLE_ATTR_PAD,
+   PSAMPLE_ATTR_OUT_TC,/* u16 */
+   PSAMPLE_ATTR_OUT_TC_OCC,/* u64, bytes */
+   PSAMPLE_ATTR_LATENCY,   /* u64, nanoseconds */
+   PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */
+   PSAMPLE_ATTR_PROTO, /* u16 */
+   PSAMPLE_ATTR_USER_COOKIE,
+
+   __PSAMPLE_ATTR_MAX
+};
+
+enum psample_command {
+   PSAMPLE_CMD_SAMP