Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Ilya Maximets 
On 7/10/24 19:17, Adrián Moreno wrote:
> On Wed, Jul 10, 2024 at 06:52:30PM GMT, Ilya Maximets wrote:
>> On 7/10/24 15:25, Adrián Moreno wrote:
>>> On Wed, Jul 10, 2024 at 02:56:44PM GMT, Ilya Maximets wrote:
 On 7/7/24 22:08, Adrian Moreno wrote:
> Use the newly added psample action to implement OpenFlow sample() actions
> with local sampling configuration if possible.
>
> A bit of refactoring in compose_sample_actions arguments helps make it a
> bit more readable.
>
> Signed-off-by: Adrian Moreno 
> ---
>  ofproto/ofproto-dpif-lsample.c |  16 +++
>  ofproto/ofproto-dpif-lsample.h |   5 +
>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
>  ofproto/ofproto-dpif-xlate.h   |   5 +-
>  ofproto/ofproto-dpif.c |   2 +-
>  tests/ofproto-dpif.at  | 146 +++
>  6 files changed, 345 insertions(+), 80 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-lsample.c 
> b/ofproto/ofproto-dpif-lsample.c
> index d675a116f..534ad96f0 100644
> --- a/ofproto/ofproto-dpif-lsample.c
> +++ b/ofproto/ofproto-dpif-lsample.c
> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample 
> *lsample,
>  return changed;
>  }
>
> +/* Returns the group_id for a given collector_set_id, if it exists. */
> +bool
> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
> collector_set_id,
> +  uint32_t *group_id)
> +{
> +struct lsample_exporter_node *node;
> +bool found = false;
> +
> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> +if (node) {
> +found = true;
> +*group_id = node->exporter.options.group_id;
> +}
> +return found;
> +}
> +
>  struct dpif_lsample *
>  dpif_lsample_create(void)
>  {
> diff --git a/ofproto/ofproto-dpif-lsample.h 
> b/ofproto/ofproto-dpif-lsample.h
> index bee36c9c5..9c1026551 100644
> --- a/ofproto/ofproto-dpif-lsample.h
> +++ b/ofproto/ofproto-dpif-lsample.h
> @@ -18,6 +18,7 @@
>  #define OFPROTO_DPIF_LSAMPLE_H 1
>
>  #include 
> +#include 
>  #include 
>
>  struct dpif_lsample;
> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
>const struct ofproto_lsample_options *,
>size_t n_opts);
>
> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> +   uint32_t collector_set_id,
> +   uint32_t *group_id);
> +
>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7c4950895..5e8113d5e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -47,6 +47,7 @@
>  #include "ofproto/ofproto-dpif-ipfix.h"
>  #include "ofproto/ofproto-dpif-mirror.h"
>  #include "ofproto/ofproto-dpif-monitor.h"
> +#include "ofproto/ofproto-dpif-lsample.h"
>  #include "ofproto/ofproto-dpif-sflow.h"
>  #include "ofproto/ofproto-dpif-trace.h"
>  #include "ofproto/ofproto-dpif-xlate-cache.h"
> @@ -117,6 +118,7 @@ struct xbridge {
>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>  struct netflow *netflow;  /* Netflow handle, or null. */
> +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>  struct stp *stp;  /* STP or null if disabled. */
>  struct rstp *rstp;/* RSTP or null if disabled. */
>
> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, 
> struct dpif *,
>const struct mbridge *,
>const struct dpif_sflow *,
>const struct dpif_ipfix *,
> +  const struct dpif_lsample *,
>const struct netflow *,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *,
> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
>const struct mbridge *mbridge,
>const struct dpif_sflow *sflow,
>const struct dpif_ipfix *ipfix,
> +  const struct dpif_lsample *lsample,
>const struct netflow *netflow,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *support,
> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
>  }
>
> +

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Adrián Moreno 
On Wed, Jul 10, 2024 at 06:52:30PM GMT, Ilya Maximets wrote:
> On 7/10/24 15:25, Adrián Moreno wrote:
> > On Wed, Jul 10, 2024 at 02:56:44PM GMT, Ilya Maximets wrote:
> >> On 7/7/24 22:08, Adrian Moreno wrote:
> >>> Use the newly added psample action to implement OpenFlow sample() actions
> >>> with local sampling configuration if possible.
> >>>
> >>> A bit of refactoring in compose_sample_actions arguments helps make it a
> >>> bit more readable.
> >>>
> >>> Signed-off-by: Adrian Moreno 
> >>> ---
> >>>  ofproto/ofproto-dpif-lsample.c |  16 +++
> >>>  ofproto/ofproto-dpif-lsample.h |   5 +
> >>>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
> >>>  ofproto/ofproto-dpif-xlate.h   |   5 +-
> >>>  ofproto/ofproto-dpif.c |   2 +-
> >>>  tests/ofproto-dpif.at  | 146 +++
> >>>  6 files changed, 345 insertions(+), 80 deletions(-)
> >>>
> >>> diff --git a/ofproto/ofproto-dpif-lsample.c 
> >>> b/ofproto/ofproto-dpif-lsample.c
> >>> index d675a116f..534ad96f0 100644
> >>> --- a/ofproto/ofproto-dpif-lsample.c
> >>> +++ b/ofproto/ofproto-dpif-lsample.c
> >>> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample 
> >>> *lsample,
> >>>  return changed;
> >>>  }
> >>>
> >>> +/* Returns the group_id for a given collector_set_id, if it exists. */
> >>> +bool
> >>> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
> >>> collector_set_id,
> >>> +  uint32_t *group_id)
> >>> +{
> >>> +struct lsample_exporter_node *node;
> >>> +bool found = false;
> >>> +
> >>> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> >>> +if (node) {
> >>> +found = true;
> >>> +*group_id = node->exporter.options.group_id;
> >>> +}
> >>> +return found;
> >>> +}
> >>> +
> >>>  struct dpif_lsample *
> >>>  dpif_lsample_create(void)
> >>>  {
> >>> diff --git a/ofproto/ofproto-dpif-lsample.h 
> >>> b/ofproto/ofproto-dpif-lsample.h
> >>> index bee36c9c5..9c1026551 100644
> >>> --- a/ofproto/ofproto-dpif-lsample.h
> >>> +++ b/ofproto/ofproto-dpif-lsample.h
> >>> @@ -18,6 +18,7 @@
> >>>  #define OFPROTO_DPIF_LSAMPLE_H 1
> >>>
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>
> >>>  struct dpif_lsample;
> >>> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
> >>>const struct ofproto_lsample_options *,
> >>>size_t n_opts);
> >>>
> >>> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> >>> +   uint32_t collector_set_id,
> >>> +   uint32_t *group_id);
> >>> +
> >>>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> >>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >>> index 7c4950895..5e8113d5e 100644
> >>> --- a/ofproto/ofproto-dpif-xlate.c
> >>> +++ b/ofproto/ofproto-dpif-xlate.c
> >>> @@ -47,6 +47,7 @@
> >>>  #include "ofproto/ofproto-dpif-ipfix.h"
> >>>  #include "ofproto/ofproto-dpif-mirror.h"
> >>>  #include "ofproto/ofproto-dpif-monitor.h"
> >>> +#include "ofproto/ofproto-dpif-lsample.h"
> >>>  #include "ofproto/ofproto-dpif-sflow.h"
> >>>  #include "ofproto/ofproto-dpif-trace.h"
> >>>  #include "ofproto/ofproto-dpif-xlate-cache.h"
> >>> @@ -117,6 +118,7 @@ struct xbridge {
> >>>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
> >>>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
> >>>  struct netflow *netflow;  /* Netflow handle, or null. */
> >>> +struct dpif_lsample *lsample; /* Local sample handle, or null. */
> >>>  struct stp *stp;  /* STP or null if disabled. */
> >>>  struct rstp *rstp;/* RSTP or null if disabled. */
> >>>
> >>> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, 
> >>> struct dpif *,
> >>>const struct mbridge *,
> >>>const struct dpif_sflow *,
> >>>const struct dpif_ipfix *,
> >>> +  const struct dpif_lsample *,
> >>>const struct netflow *,
> >>>bool forward_bpdu, bool has_in_band,
> >>>const struct dpif_backer_support *,
> >>> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >>>const struct mbridge *mbridge,
> >>>const struct dpif_sflow *sflow,
> >>>const struct dpif_ipfix *ipfix,
> >>> +  const struct dpif_lsample *lsample,
> >>>const struct netflow *netflow,
> >>>bool forward_bpdu, bool has_in_band,
> >>>const struct dpif_backer_support *support,
> >>> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >>>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
> >>>  }
> >>>
> >>> +if (xbridge->lsample != lsample) {
> >>

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Ilya Maximets 
On 7/10/24 15:25, Adrián Moreno wrote:
> On Wed, Jul 10, 2024 at 02:56:44PM GMT, Ilya Maximets wrote:
>> On 7/7/24 22:08, Adrian Moreno wrote:
>>> Use the newly added psample action to implement OpenFlow sample() actions
>>> with local sampling configuration if possible.
>>>
>>> A bit of refactoring in compose_sample_actions arguments helps make it a
>>> bit more readable.
>>>
>>> Signed-off-by: Adrian Moreno 
>>> ---
>>>  ofproto/ofproto-dpif-lsample.c |  16 +++
>>>  ofproto/ofproto-dpif-lsample.h |   5 +
>>>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
>>>  ofproto/ofproto-dpif-xlate.h   |   5 +-
>>>  ofproto/ofproto-dpif.c |   2 +-
>>>  tests/ofproto-dpif.at  | 146 +++
>>>  6 files changed, 345 insertions(+), 80 deletions(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
>>> index d675a116f..534ad96f0 100644
>>> --- a/ofproto/ofproto-dpif-lsample.c
>>> +++ b/ofproto/ofproto-dpif-lsample.c
>>> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
>>>  return changed;
>>>  }
>>>
>>> +/* Returns the group_id for a given collector_set_id, if it exists. */
>>> +bool
>>> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
>>> collector_set_id,
>>> +  uint32_t *group_id)
>>> +{
>>> +struct lsample_exporter_node *node;
>>> +bool found = false;
>>> +
>>> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
>>> +if (node) {
>>> +found = true;
>>> +*group_id = node->exporter.options.group_id;
>>> +}
>>> +return found;
>>> +}
>>> +
>>>  struct dpif_lsample *
>>>  dpif_lsample_create(void)
>>>  {
>>> diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
>>> index bee36c9c5..9c1026551 100644
>>> --- a/ofproto/ofproto-dpif-lsample.h
>>> +++ b/ofproto/ofproto-dpif-lsample.h
>>> @@ -18,6 +18,7 @@
>>>  #define OFPROTO_DPIF_LSAMPLE_H 1
>>>
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  struct dpif_lsample;
>>> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
>>>const struct ofproto_lsample_options *,
>>>size_t n_opts);
>>>
>>> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
>>> +   uint32_t collector_set_id,
>>> +   uint32_t *group_id);
>>> +
>>>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>> index 7c4950895..5e8113d5e 100644
>>> --- a/ofproto/ofproto-dpif-xlate.c
>>> +++ b/ofproto/ofproto-dpif-xlate.c
>>> @@ -47,6 +47,7 @@
>>>  #include "ofproto/ofproto-dpif-ipfix.h"
>>>  #include "ofproto/ofproto-dpif-mirror.h"
>>>  #include "ofproto/ofproto-dpif-monitor.h"
>>> +#include "ofproto/ofproto-dpif-lsample.h"
>>>  #include "ofproto/ofproto-dpif-sflow.h"
>>>  #include "ofproto/ofproto-dpif-trace.h"
>>>  #include "ofproto/ofproto-dpif-xlate-cache.h"
>>> @@ -117,6 +118,7 @@ struct xbridge {
>>>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
>>>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>>>  struct netflow *netflow;  /* Netflow handle, or null. */
>>> +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>>>  struct stp *stp;  /* STP or null if disabled. */
>>>  struct rstp *rstp;/* RSTP or null if disabled. */
>>>
>>> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
>>> dpif *,
>>>const struct mbridge *,
>>>const struct dpif_sflow *,
>>>const struct dpif_ipfix *,
>>> +  const struct dpif_lsample *,
>>>const struct netflow *,
>>>bool forward_bpdu, bool has_in_band,
>>>const struct dpif_backer_support *,
>>> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
>>>const struct mbridge *mbridge,
>>>const struct dpif_sflow *sflow,
>>>const struct dpif_ipfix *ipfix,
>>> +  const struct dpif_lsample *lsample,
>>>const struct netflow *netflow,
>>>bool forward_bpdu, bool has_in_band,
>>>const struct dpif_backer_support *support,
>>> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>>>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
>>>  }
>>>
>>> +if (xbridge->lsample != lsample) {
>>> +dpif_lsample_unref(xbridge->lsample);
>>> +xbridge->lsample = dpif_lsample_ref(lsample);
>>> +}
>>> +
>>>  if (xbridge->stp != stp) {
>>>  stp_unref(xbridge->stp);
>>>  xbridge->stp = stp_ref(stp);
>>> @@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *x

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Adrián Moreno 
On Wed, Jul 10, 2024 at 02:56:44PM GMT, Ilya Maximets wrote:
> On 7/7/24 22:08, Adrian Moreno wrote:
> > Use the newly added psample action to implement OpenFlow sample() actions
> > with local sampling configuration if possible.
> >
> > A bit of refactoring in compose_sample_actions arguments helps make it a
> > bit more readable.
> >
> > Signed-off-by: Adrian Moreno 
> > ---
> >  ofproto/ofproto-dpif-lsample.c |  16 +++
> >  ofproto/ofproto-dpif-lsample.h |   5 +
> >  ofproto/ofproto-dpif-xlate.c   | 251 +++--
> >  ofproto/ofproto-dpif-xlate.h   |   5 +-
> >  ofproto/ofproto-dpif.c |   2 +-
> >  tests/ofproto-dpif.at  | 146 +++
> >  6 files changed, 345 insertions(+), 80 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
> > index d675a116f..534ad96f0 100644
> > --- a/ofproto/ofproto-dpif-lsample.c
> > +++ b/ofproto/ofproto-dpif-lsample.c
> > @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
> >  return changed;
> >  }
> >
> > +/* Returns the group_id for a given collector_set_id, if it exists. */
> > +bool
> > +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
> > collector_set_id,
> > +  uint32_t *group_id)
> > +{
> > +struct lsample_exporter_node *node;
> > +bool found = false;
> > +
> > +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> > +if (node) {
> > +found = true;
> > +*group_id = node->exporter.options.group_id;
> > +}
> > +return found;
> > +}
> > +
> >  struct dpif_lsample *
> >  dpif_lsample_create(void)
> >  {
> > diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
> > index bee36c9c5..9c1026551 100644
> > --- a/ofproto/ofproto-dpif-lsample.h
> > +++ b/ofproto/ofproto-dpif-lsample.h
> > @@ -18,6 +18,7 @@
> >  #define OFPROTO_DPIF_LSAMPLE_H 1
> >
> >  #include 
> > +#include 
> >  #include 
> >
> >  struct dpif_lsample;
> > @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
> >const struct ofproto_lsample_options *,
> >size_t n_opts);
> >
> > +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> > +   uint32_t collector_set_id,
> > +   uint32_t *group_id);
> > +
> >  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 7c4950895..5e8113d5e 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -47,6 +47,7 @@
> >  #include "ofproto/ofproto-dpif-ipfix.h"
> >  #include "ofproto/ofproto-dpif-mirror.h"
> >  #include "ofproto/ofproto-dpif-monitor.h"
> > +#include "ofproto/ofproto-dpif-lsample.h"
> >  #include "ofproto/ofproto-dpif-sflow.h"
> >  #include "ofproto/ofproto-dpif-trace.h"
> >  #include "ofproto/ofproto-dpif-xlate-cache.h"
> > @@ -117,6 +118,7 @@ struct xbridge {
> >  struct dpif_sflow *sflow; /* SFlow handle, or null. */
> >  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
> >  struct netflow *netflow;  /* Netflow handle, or null. */
> > +struct dpif_lsample *lsample; /* Local sample handle, or null. */
> >  struct stp *stp;  /* STP or null if disabled. */
> >  struct rstp *rstp;/* RSTP or null if disabled. */
> >
> > @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
> > dpif *,
> >const struct mbridge *,
> >const struct dpif_sflow *,
> >const struct dpif_ipfix *,
> > +  const struct dpif_lsample *,
> >const struct netflow *,
> >bool forward_bpdu, bool has_in_band,
> >const struct dpif_backer_support *,
> > @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >const struct mbridge *mbridge,
> >const struct dpif_sflow *sflow,
> >const struct dpif_ipfix *ipfix,
> > +  const struct dpif_lsample *lsample,
> >const struct netflow *netflow,
> >bool forward_bpdu, bool has_in_band,
> >const struct dpif_backer_support *support,
> > @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >  xbridge->ipfix = dpif_ipfix_ref(ipfix);
> >  }
> >
> > +if (xbridge->lsample != lsample) {
> > +dpif_lsample_unref(xbridge->lsample);
> > +xbridge->lsample = dpif_lsample_ref(lsample);
> > +}
> > +
> >  if (xbridge->stp != stp) {
> >  stp_unref(xbridge->stp);
> >  xbridge->stp = stp_ref(stp);
> > @@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *xbridge)
> >  xlate_xbridge_set(new_xbr

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Ilya Maximets 
On 7/7/24 22:08, Adrian Moreno wrote:
> Use the newly added psample action to implement OpenFlow sample() actions
> with local sampling configuration if possible.
> 
> A bit of refactoring in compose_sample_actions arguments helps make it a
> bit more readable.
> 
> Signed-off-by: Adrian Moreno 
> ---
>  ofproto/ofproto-dpif-lsample.c |  16 +++
>  ofproto/ofproto-dpif-lsample.h |   5 +
>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
>  ofproto/ofproto-dpif-xlate.h   |   5 +-
>  ofproto/ofproto-dpif.c |   2 +-
>  tests/ofproto-dpif.at  | 146 +++
>  6 files changed, 345 insertions(+), 80 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
> index d675a116f..534ad96f0 100644
> --- a/ofproto/ofproto-dpif-lsample.c
> +++ b/ofproto/ofproto-dpif-lsample.c
> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
>  return changed;
>  }
>  
> +/* Returns the group_id for a given collector_set_id, if it exists. */
> +bool
> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t collector_set_id,
> +  uint32_t *group_id)
> +{
> +struct lsample_exporter_node *node;
> +bool found = false;
> +
> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> +if (node) {
> +found = true;
> +*group_id = node->exporter.options.group_id;
> +}
> +return found;
> +}
> +
>  struct dpif_lsample *
>  dpif_lsample_create(void)
>  {
> diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
> index bee36c9c5..9c1026551 100644
> --- a/ofproto/ofproto-dpif-lsample.h
> +++ b/ofproto/ofproto-dpif-lsample.h
> @@ -18,6 +18,7 @@
>  #define OFPROTO_DPIF_LSAMPLE_H 1
>  
>  #include 
> +#include 
>  #include 
>  
>  struct dpif_lsample;
> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
>const struct ofproto_lsample_options *,
>size_t n_opts);
>  
> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> +   uint32_t collector_set_id,
> +   uint32_t *group_id);
> +
>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7c4950895..5e8113d5e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -47,6 +47,7 @@
>  #include "ofproto/ofproto-dpif-ipfix.h"
>  #include "ofproto/ofproto-dpif-mirror.h"
>  #include "ofproto/ofproto-dpif-monitor.h"
> +#include "ofproto/ofproto-dpif-lsample.h"
>  #include "ofproto/ofproto-dpif-sflow.h"
>  #include "ofproto/ofproto-dpif-trace.h"
>  #include "ofproto/ofproto-dpif-xlate-cache.h"
> @@ -117,6 +118,7 @@ struct xbridge {
>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>  struct netflow *netflow;  /* Netflow handle, or null. */
> +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>  struct stp *stp;  /* STP or null if disabled. */
>  struct rstp *rstp;/* RSTP or null if disabled. */
>  
> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
> dpif *,
>const struct mbridge *,
>const struct dpif_sflow *,
>const struct dpif_ipfix *,
> +  const struct dpif_lsample *,
>const struct netflow *,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *,
> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
>const struct mbridge *mbridge,
>const struct dpif_sflow *sflow,
>const struct dpif_ipfix *ipfix,
> +  const struct dpif_lsample *lsample,
>const struct netflow *netflow,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *support,
> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
>  }
>  
> +if (xbridge->lsample != lsample) {
> +dpif_lsample_unref(xbridge->lsample);
> +xbridge->lsample = dpif_lsample_ref(lsample);
> +}
> +
>  if (xbridge->stp != stp) {
>  stp_unref(xbridge->stp);
>  xbridge->stp = stp_ref(stp);
> @@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *xbridge)
>  xlate_xbridge_set(new_xbridge,
>xbridge->dpif, xbridge->ml, xbridge->stp,
>xbridge->rstp, xbridge->ms, xbridge->mbridge,
> -  xbridge->sflow, xbridge->ipfix, xbridge->netflow,
> -  xbridge->forward_bpdu, xbridge->has_in_

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Adrián Moreno 
On Wed, Jul 10, 2024 at 02:18:35PM GMT, Ilya Maximets wrote:
> On 7/9/24 16:03, Eelco Chaudron wrote:
> >
> >
> > On 9 Jul 2024, at 15:52, Adrián Moreno wrote:
> >
> >> On Tue, Jul 09, 2024 at 11:45:51AM GMT, Eelco Chaudron wrote:
> >>> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
> >>>
>  Use the newly added psample action to implement OpenFlow sample() actions
>  with local sampling configuration if possible.
> 
>  A bit of refactoring in compose_sample_actions arguments helps make it a
>  bit more readable.
> >>>
> >>> Some comments below.
> >>>
> >>> Cheers,
> >>>
> >>> Eelco
> >>>
>  Signed-off-by: Adrian Moreno 
>  ---
>   ofproto/ofproto-dpif-lsample.c |  16 +++
>   ofproto/ofproto-dpif-lsample.h |   5 +
>   ofproto/ofproto-dpif-xlate.c   | 251 +++--
>   ofproto/ofproto-dpif-xlate.h   |   5 +-
>   ofproto/ofproto-dpif.c |   2 +-
>   tests/ofproto-dpif.at  | 146 +++
>   6 files changed, 345 insertions(+), 80 deletions(-)
> 
>  diff --git a/ofproto/ofproto-dpif-lsample.c 
>  b/ofproto/ofproto-dpif-lsample.c
>  index d675a116f..534ad96f0 100644
>  --- a/ofproto/ofproto-dpif-lsample.c
>  +++ b/ofproto/ofproto-dpif-lsample.c
>  @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample 
>  *lsample,
>   return changed;
>   }
> 
>  +/* Returns the group_id for a given collector_set_id, if it exists. */
>  +bool
>  +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
>  collector_set_id,
>  +  uint32_t *group_id)
>  +{
>  +struct lsample_exporter_node *node;
>  +bool found = false;
>  +
>  +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
>  +if (node) {
>  +found = true;
>  +*group_id = node->exporter.options.group_id;
>  +}
>  +return found;
>  +}
>  +
>   struct dpif_lsample *
>   dpif_lsample_create(void)
>   {
>  diff --git a/ofproto/ofproto-dpif-lsample.h 
>  b/ofproto/ofproto-dpif-lsample.h
>  index bee36c9c5..9c1026551 100644
>  --- a/ofproto/ofproto-dpif-lsample.h
>  +++ b/ofproto/ofproto-dpif-lsample.h
>  @@ -18,6 +18,7 @@
>   #define OFPROTO_DPIF_LSAMPLE_H 1
> 
>   #include 
>  +#include 
>   #include 
> 
>   struct dpif_lsample;
>  @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
> const struct ofproto_lsample_options *,
> size_t n_opts);
> 
>  +bool dpif_lsample_get_group_id(struct dpif_lsample *,
>  +   uint32_t collector_set_id,
>  +   uint32_t *group_id);
>  +
>   #endif /* OFPROTO_DPIF_LSAMPLE_H */
>  diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>  index 7c4950895..5e8113d5e 100644
>  --- a/ofproto/ofproto-dpif-xlate.c
>  +++ b/ofproto/ofproto-dpif-xlate.c
>  @@ -47,6 +47,7 @@
>   #include "ofproto/ofproto-dpif-ipfix.h"
>   #include "ofproto/ofproto-dpif-mirror.h"
>   #include "ofproto/ofproto-dpif-monitor.h"
>  +#include "ofproto/ofproto-dpif-lsample.h"
> >>>
> >>> Add in alphabetical order?
> >>
> >> Ack.
> >>
> >>>
>   #include "ofproto/ofproto-dpif-sflow.h"
>   #include "ofproto/ofproto-dpif-trace.h"
>   #include "ofproto/ofproto-dpif-xlate-cache.h"
>  @@ -117,6 +118,7 @@ struct xbridge {
>   struct dpif_sflow *sflow; /* SFlow handle, or null. */
>   struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>   struct netflow *netflow;  /* Netflow handle, or null. */
>  +struct dpif_lsample *lsample; /* Local sample handle, or null. */
> >>>
> >>> I would move above netflow, as you also do the actual init/unref before 
> >>> netflow.
> >>
> >>
> >> Ack.
> >>
> >>
> >>>
>   struct stp *stp;  /* STP or null if disabled. */
>   struct rstp *rstp;/* RSTP or null if disabled. */
> 
>  @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, 
>  struct dpif *,
> const struct mbridge *,
> const struct dpif_sflow *,
> const struct dpif_ipfix *,
>  +  const struct dpif_lsample *,
> const struct netflow *,
> bool forward_bpdu, bool has_in_band,
> const struct dpif_backer_support *,
>  @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
> const struct mbridge *mbridge,
> const struct dpif_sflow *sflow,
> const struct dpif_ipfix

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-10 Thread Ilya Maximets 
On 7/9/24 16:03, Eelco Chaudron wrote:
> 
> 
> On 9 Jul 2024, at 15:52, Adrián Moreno wrote:
> 
>> On Tue, Jul 09, 2024 at 11:45:51AM GMT, Eelco Chaudron wrote:
>>> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
>>>
 Use the newly added psample action to implement OpenFlow sample() actions
 with local sampling configuration if possible.

 A bit of refactoring in compose_sample_actions arguments helps make it a
 bit more readable.
>>>
>>> Some comments below.
>>>
>>> Cheers,
>>>
>>> Eelco
>>>
 Signed-off-by: Adrian Moreno 
 ---
  ofproto/ofproto-dpif-lsample.c |  16 +++
  ofproto/ofproto-dpif-lsample.h |   5 +
  ofproto/ofproto-dpif-xlate.c   | 251 +++--
  ofproto/ofproto-dpif-xlate.h   |   5 +-
  ofproto/ofproto-dpif.c |   2 +-
  tests/ofproto-dpif.at  | 146 +++
  6 files changed, 345 insertions(+), 80 deletions(-)

 diff --git a/ofproto/ofproto-dpif-lsample.c 
 b/ofproto/ofproto-dpif-lsample.c
 index d675a116f..534ad96f0 100644
 --- a/ofproto/ofproto-dpif-lsample.c
 +++ b/ofproto/ofproto-dpif-lsample.c
 @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
  return changed;
  }

 +/* Returns the group_id for a given collector_set_id, if it exists. */
 +bool
 +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
 collector_set_id,
 +  uint32_t *group_id)
 +{
 +struct lsample_exporter_node *node;
 +bool found = false;
 +
 +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
 +if (node) {
 +found = true;
 +*group_id = node->exporter.options.group_id;
 +}
 +return found;
 +}
 +
  struct dpif_lsample *
  dpif_lsample_create(void)
  {
 diff --git a/ofproto/ofproto-dpif-lsample.h 
 b/ofproto/ofproto-dpif-lsample.h
 index bee36c9c5..9c1026551 100644
 --- a/ofproto/ofproto-dpif-lsample.h
 +++ b/ofproto/ofproto-dpif-lsample.h
 @@ -18,6 +18,7 @@
  #define OFPROTO_DPIF_LSAMPLE_H 1

  #include 
 +#include 
  #include 

  struct dpif_lsample;
 @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
const struct ofproto_lsample_options *,
size_t n_opts);

 +bool dpif_lsample_get_group_id(struct dpif_lsample *,
 +   uint32_t collector_set_id,
 +   uint32_t *group_id);
 +
  #endif /* OFPROTO_DPIF_LSAMPLE_H */
 diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
 index 7c4950895..5e8113d5e 100644
 --- a/ofproto/ofproto-dpif-xlate.c
 +++ b/ofproto/ofproto-dpif-xlate.c
 @@ -47,6 +47,7 @@
  #include "ofproto/ofproto-dpif-ipfix.h"
  #include "ofproto/ofproto-dpif-mirror.h"
  #include "ofproto/ofproto-dpif-monitor.h"
 +#include "ofproto/ofproto-dpif-lsample.h"
>>>
>>> Add in alphabetical order?
>>
>> Ack.
>>
>>>
  #include "ofproto/ofproto-dpif-sflow.h"
  #include "ofproto/ofproto-dpif-trace.h"
  #include "ofproto/ofproto-dpif-xlate-cache.h"
 @@ -117,6 +118,7 @@ struct xbridge {
  struct dpif_sflow *sflow; /* SFlow handle, or null. */
  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
  struct netflow *netflow;  /* Netflow handle, or null. */
 +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>>>
>>> I would move above netflow, as you also do the actual init/unref before 
>>> netflow.
>>
>>
>> Ack.
>>
>>
>>>
  struct stp *stp;  /* STP or null if disabled. */
  struct rstp *rstp;/* RSTP or null if disabled. */

 @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
 dpif *,
const struct mbridge *,
const struct dpif_sflow *,
const struct dpif_ipfix *,
 +  const struct dpif_lsample *,
const struct netflow *,
bool forward_bpdu, bool has_in_band,
const struct dpif_backer_support *,
 @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
const struct mbridge *mbridge,
const struct dpif_sflow *sflow,
const struct dpif_ipfix *ipfix,
 +  const struct dpif_lsample *lsample,
const struct netflow *netflow,
bool forward_bpdu, bool has_in_band,
const struct dpif_backer_support *support,
 @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>>

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-09 Thread Eelco Chaudron 


On 9 Jul 2024, at 15:52, Adrián Moreno wrote:

> On Tue, Jul 09, 2024 at 11:45:51AM GMT, Eelco Chaudron wrote:
>> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
>>
>>> Use the newly added psample action to implement OpenFlow sample() actions
>>> with local sampling configuration if possible.
>>>
>>> A bit of refactoring in compose_sample_actions arguments helps make it a
>>> bit more readable.
>>
>> Some comments below.
>>
>> Cheers,
>>
>> Eelco
>>
>>> Signed-off-by: Adrian Moreno 
>>> ---
>>>  ofproto/ofproto-dpif-lsample.c |  16 +++
>>>  ofproto/ofproto-dpif-lsample.h |   5 +
>>>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
>>>  ofproto/ofproto-dpif-xlate.h   |   5 +-
>>>  ofproto/ofproto-dpif.c |   2 +-
>>>  tests/ofproto-dpif.at  | 146 +++
>>>  6 files changed, 345 insertions(+), 80 deletions(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
>>> index d675a116f..534ad96f0 100644
>>> --- a/ofproto/ofproto-dpif-lsample.c
>>> +++ b/ofproto/ofproto-dpif-lsample.c
>>> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
>>>  return changed;
>>>  }
>>>
>>> +/* Returns the group_id for a given collector_set_id, if it exists. */
>>> +bool
>>> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
>>> collector_set_id,
>>> +  uint32_t *group_id)
>>> +{
>>> +struct lsample_exporter_node *node;
>>> +bool found = false;
>>> +
>>> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
>>> +if (node) {
>>> +found = true;
>>> +*group_id = node->exporter.options.group_id;
>>> +}
>>> +return found;
>>> +}
>>> +
>>>  struct dpif_lsample *
>>>  dpif_lsample_create(void)
>>>  {
>>> diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
>>> index bee36c9c5..9c1026551 100644
>>> --- a/ofproto/ofproto-dpif-lsample.h
>>> +++ b/ofproto/ofproto-dpif-lsample.h
>>> @@ -18,6 +18,7 @@
>>>  #define OFPROTO_DPIF_LSAMPLE_H 1
>>>
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  struct dpif_lsample;
>>> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
>>>const struct ofproto_lsample_options *,
>>>size_t n_opts);
>>>
>>> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
>>> +   uint32_t collector_set_id,
>>> +   uint32_t *group_id);
>>> +
>>>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>> index 7c4950895..5e8113d5e 100644
>>> --- a/ofproto/ofproto-dpif-xlate.c
>>> +++ b/ofproto/ofproto-dpif-xlate.c
>>> @@ -47,6 +47,7 @@
>>>  #include "ofproto/ofproto-dpif-ipfix.h"
>>>  #include "ofproto/ofproto-dpif-mirror.h"
>>>  #include "ofproto/ofproto-dpif-monitor.h"
>>> +#include "ofproto/ofproto-dpif-lsample.h"
>>
>> Add in alphabetical order?
>
> Ack.
>
>>
>>>  #include "ofproto/ofproto-dpif-sflow.h"
>>>  #include "ofproto/ofproto-dpif-trace.h"
>>>  #include "ofproto/ofproto-dpif-xlate-cache.h"
>>> @@ -117,6 +118,7 @@ struct xbridge {
>>>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
>>>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>>>  struct netflow *netflow;  /* Netflow handle, or null. */
>>> +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>>
>> I would move above netflow, as you also do the actual init/unref before 
>> netflow.
>
>
> Ack.
>
>
>>
>>>  struct stp *stp;  /* STP or null if disabled. */
>>>  struct rstp *rstp;/* RSTP or null if disabled. */
>>>
>>> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
>>> dpif *,
>>>const struct mbridge *,
>>>const struct dpif_sflow *,
>>>const struct dpif_ipfix *,
>>> +  const struct dpif_lsample *,
>>>const struct netflow *,
>>>bool forward_bpdu, bool has_in_band,
>>>const struct dpif_backer_support *,
>>> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
>>>const struct mbridge *mbridge,
>>>const struct dpif_sflow *sflow,
>>>const struct dpif_ipfix *ipfix,
>>> +  const struct dpif_lsample *lsample,
>>>const struct netflow *netflow,
>>>bool forward_bpdu, bool has_in_band,
>>>const struct dpif_backer_support *support,
>>> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>>>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
>>>  }
>>>
>>> +if (xbridge->lsample != lsample) {
>>> +dpif_lsample_unref(xbridge->lsample);
>>> +xbridge->lsa

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-09 Thread Adrián Moreno 
On Tue, Jul 09, 2024 at 11:45:51AM GMT, Eelco Chaudron wrote:
> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
>
> > Use the newly added psample action to implement OpenFlow sample() actions
> > with local sampling configuration if possible.
> >
> > A bit of refactoring in compose_sample_actions arguments helps make it a
> > bit more readable.
>
> Some comments below.
>
> Cheers,
>
> Eelco
>
> > Signed-off-by: Adrian Moreno 
> > ---
> >  ofproto/ofproto-dpif-lsample.c |  16 +++
> >  ofproto/ofproto-dpif-lsample.h |   5 +
> >  ofproto/ofproto-dpif-xlate.c   | 251 +++--
> >  ofproto/ofproto-dpif-xlate.h   |   5 +-
> >  ofproto/ofproto-dpif.c |   2 +-
> >  tests/ofproto-dpif.at  | 146 +++
> >  6 files changed, 345 insertions(+), 80 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
> > index d675a116f..534ad96f0 100644
> > --- a/ofproto/ofproto-dpif-lsample.c
> > +++ b/ofproto/ofproto-dpif-lsample.c
> > @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
> >  return changed;
> >  }
> >
> > +/* Returns the group_id for a given collector_set_id, if it exists. */
> > +bool
> > +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t 
> > collector_set_id,
> > +  uint32_t *group_id)
> > +{
> > +struct lsample_exporter_node *node;
> > +bool found = false;
> > +
> > +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> > +if (node) {
> > +found = true;
> > +*group_id = node->exporter.options.group_id;
> > +}
> > +return found;
> > +}
> > +
> >  struct dpif_lsample *
> >  dpif_lsample_create(void)
> >  {
> > diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
> > index bee36c9c5..9c1026551 100644
> > --- a/ofproto/ofproto-dpif-lsample.h
> > +++ b/ofproto/ofproto-dpif-lsample.h
> > @@ -18,6 +18,7 @@
> >  #define OFPROTO_DPIF_LSAMPLE_H 1
> >
> >  #include 
> > +#include 
> >  #include 
> >
> >  struct dpif_lsample;
> > @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
> >const struct ofproto_lsample_options *,
> >size_t n_opts);
> >
> > +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> > +   uint32_t collector_set_id,
> > +   uint32_t *group_id);
> > +
> >  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 7c4950895..5e8113d5e 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -47,6 +47,7 @@
> >  #include "ofproto/ofproto-dpif-ipfix.h"
> >  #include "ofproto/ofproto-dpif-mirror.h"
> >  #include "ofproto/ofproto-dpif-monitor.h"
> > +#include "ofproto/ofproto-dpif-lsample.h"
>
> Add in alphabetical order?

Ack.

>
> >  #include "ofproto/ofproto-dpif-sflow.h"
> >  #include "ofproto/ofproto-dpif-trace.h"
> >  #include "ofproto/ofproto-dpif-xlate-cache.h"
> > @@ -117,6 +118,7 @@ struct xbridge {
> >  struct dpif_sflow *sflow; /* SFlow handle, or null. */
> >  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
> >  struct netflow *netflow;  /* Netflow handle, or null. */
> > +struct dpif_lsample *lsample; /* Local sample handle, or null. */
>
> I would move above netflow, as you also do the actual init/unref before 
> netflow.


Ack.


>
> >  struct stp *stp;  /* STP or null if disabled. */
> >  struct rstp *rstp;/* RSTP or null if disabled. */
> >
> > @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
> > dpif *,
> >const struct mbridge *,
> >const struct dpif_sflow *,
> >const struct dpif_ipfix *,
> > +  const struct dpif_lsample *,
> >const struct netflow *,
> >bool forward_bpdu, bool has_in_band,
> >const struct dpif_backer_support *,
> > @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >const struct mbridge *mbridge,
> >const struct dpif_sflow *sflow,
> >const struct dpif_ipfix *ipfix,
> > +  const struct dpif_lsample *lsample,
> >const struct netflow *netflow,
> >bool forward_bpdu, bool has_in_band,
> >const struct dpif_backer_support *support,
> > @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >  xbridge->ipfix = dpif_ipfix_ref(ipfix);
> >  }
> >
> > +if (xbridge->lsample != lsample) {
> > +dpif_lsample_unref(xbridge->lsample);
> > +xbridge->lsample = dpif_lsample_ref(lsample);
> > +}
> > +
> >  if (xbridge->stp

Re: [ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-09 Thread Eelco Chaudron 
On 7 Jul 2024, at 22:08, Adrian Moreno wrote:

> Use the newly added psample action to implement OpenFlow sample() actions
> with local sampling configuration if possible.
>
> A bit of refactoring in compose_sample_actions arguments helps make it a
> bit more readable.

Some comments below.

Cheers,

Eelco

> Signed-off-by: Adrian Moreno 
> ---
>  ofproto/ofproto-dpif-lsample.c |  16 +++
>  ofproto/ofproto-dpif-lsample.h |   5 +
>  ofproto/ofproto-dpif-xlate.c   | 251 +++--
>  ofproto/ofproto-dpif-xlate.h   |   5 +-
>  ofproto/ofproto-dpif.c |   2 +-
>  tests/ofproto-dpif.at  | 146 +++
>  6 files changed, 345 insertions(+), 80 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
> index d675a116f..534ad96f0 100644
> --- a/ofproto/ofproto-dpif-lsample.c
> +++ b/ofproto/ofproto-dpif-lsample.c
> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
>  return changed;
>  }
>
> +/* Returns the group_id for a given collector_set_id, if it exists. */
> +bool
> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t collector_set_id,
> +  uint32_t *group_id)
> +{
> +struct lsample_exporter_node *node;
> +bool found = false;
> +
> +node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> +if (node) {
> +found = true;
> +*group_id = node->exporter.options.group_id;
> +}
> +return found;
> +}
> +
>  struct dpif_lsample *
>  dpif_lsample_create(void)
>  {
> diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
> index bee36c9c5..9c1026551 100644
> --- a/ofproto/ofproto-dpif-lsample.h
> +++ b/ofproto/ofproto-dpif-lsample.h
> @@ -18,6 +18,7 @@
>  #define OFPROTO_DPIF_LSAMPLE_H 1
>
>  #include 
> +#include 
>  #include 
>
>  struct dpif_lsample;
> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
>const struct ofproto_lsample_options *,
>size_t n_opts);
>
> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> +   uint32_t collector_set_id,
> +   uint32_t *group_id);
> +
>  #endif /* OFPROTO_DPIF_LSAMPLE_H */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7c4950895..5e8113d5e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -47,6 +47,7 @@
>  #include "ofproto/ofproto-dpif-ipfix.h"
>  #include "ofproto/ofproto-dpif-mirror.h"
>  #include "ofproto/ofproto-dpif-monitor.h"
> +#include "ofproto/ofproto-dpif-lsample.h"

Add in alphabetical order?

>  #include "ofproto/ofproto-dpif-sflow.h"
>  #include "ofproto/ofproto-dpif-trace.h"
>  #include "ofproto/ofproto-dpif-xlate-cache.h"
> @@ -117,6 +118,7 @@ struct xbridge {
>  struct dpif_sflow *sflow; /* SFlow handle, or null. */
>  struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
>  struct netflow *netflow;  /* Netflow handle, or null. */
> +struct dpif_lsample *lsample; /* Local sample handle, or null. */

I would move above netflow, as you also do the actual init/unref before netflow.

>  struct stp *stp;  /* STP or null if disabled. */
>  struct rstp *rstp;/* RSTP or null if disabled. */
>
> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct 
> dpif *,
>const struct mbridge *,
>const struct dpif_sflow *,
>const struct dpif_ipfix *,
> +  const struct dpif_lsample *,
>const struct netflow *,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *,
> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
>const struct mbridge *mbridge,
>const struct dpif_sflow *sflow,
>const struct dpif_ipfix *ipfix,
> +  const struct dpif_lsample *lsample,
>const struct netflow *netflow,
>bool forward_bpdu, bool has_in_band,
>const struct dpif_backer_support *support,
> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
>  xbridge->ipfix = dpif_ipfix_ref(ipfix);
>  }
>
> +if (xbridge->lsample != lsample) {
> +dpif_lsample_unref(xbridge->lsample);
> +xbridge->lsample = dpif_lsample_ref(lsample);
> +}
> +
>  if (xbridge->stp != stp) {
>  stp_unref(xbridge->stp);
>  xbridge->stp = stp_ref(stp);
> @@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *xbridge)
>  xlate_xbridge_set(new_xbridge,
>xbridge->dpif, xbridge->ml, xbridge->stp,
>xbridge->rstp, xbridge->ms, xbridge->mbrid

[ovs-dev] [PATCH v1 06/13] ofproto-dpif-xlate: Use psample for local sample.

2024-07-07 Thread Adrian Moreno 
Use the newly added psample action to implement OpenFlow sample() actions
with local sampling configuration if possible.

A bit of refactoring in compose_sample_actions arguments helps make it a
bit more readable.

Signed-off-by: Adrian Moreno 
---
 ofproto/ofproto-dpif-lsample.c |  16 +++
 ofproto/ofproto-dpif-lsample.h |   5 +
 ofproto/ofproto-dpif-xlate.c   | 251 +++--
 ofproto/ofproto-dpif-xlate.h   |   5 +-
 ofproto/ofproto-dpif.c |   2 +-
 tests/ofproto-dpif.at  | 146 +++
 6 files changed, 345 insertions(+), 80 deletions(-)

diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
index d675a116f..534ad96f0 100644
--- a/ofproto/ofproto-dpif-lsample.c
+++ b/ofproto/ofproto-dpif-lsample.c
@@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample *lsample,
 return changed;
 }
 
+/* Returns the group_id for a given collector_set_id, if it exists. */
+bool
+dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t collector_set_id,
+  uint32_t *group_id)
+{
+struct lsample_exporter_node *node;
+bool found = false;
+
+node = dpif_lsample_find_exporter_node(ps, collector_set_id);
+if (node) {
+found = true;
+*group_id = node->exporter.options.group_id;
+}
+return found;
+}
+
 struct dpif_lsample *
 dpif_lsample_create(void)
 {
diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
index bee36c9c5..9c1026551 100644
--- a/ofproto/ofproto-dpif-lsample.h
+++ b/ofproto/ofproto-dpif-lsample.h
@@ -18,6 +18,7 @@
 #define OFPROTO_DPIF_LSAMPLE_H 1
 
 #include 
+#include 
 #include 
 
 struct dpif_lsample;
@@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
   const struct ofproto_lsample_options *,
   size_t n_opts);
 
+bool dpif_lsample_get_group_id(struct dpif_lsample *,
+   uint32_t collector_set_id,
+   uint32_t *group_id);
+
 #endif /* OFPROTO_DPIF_LSAMPLE_H */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7c4950895..5e8113d5e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -47,6 +47,7 @@
 #include "ofproto/ofproto-dpif-ipfix.h"
 #include "ofproto/ofproto-dpif-mirror.h"
 #include "ofproto/ofproto-dpif-monitor.h"
+#include "ofproto/ofproto-dpif-lsample.h"
 #include "ofproto/ofproto-dpif-sflow.h"
 #include "ofproto/ofproto-dpif-trace.h"
 #include "ofproto/ofproto-dpif-xlate-cache.h"
@@ -117,6 +118,7 @@ struct xbridge {
 struct dpif_sflow *sflow; /* SFlow handle, or null. */
 struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
 struct netflow *netflow;  /* Netflow handle, or null. */
+struct dpif_lsample *lsample; /* Local sample handle, or null. */
 struct stp *stp;  /* STP or null if disabled. */
 struct rstp *rstp;/* RSTP or null if disabled. */
 
@@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *, struct dpif 
*,
   const struct mbridge *,
   const struct dpif_sflow *,
   const struct dpif_ipfix *,
+  const struct dpif_lsample *,
   const struct netflow *,
   bool forward_bpdu, bool has_in_band,
   const struct dpif_backer_support *,
@@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
   const struct mbridge *mbridge,
   const struct dpif_sflow *sflow,
   const struct dpif_ipfix *ipfix,
+  const struct dpif_lsample *lsample,
   const struct netflow *netflow,
   bool forward_bpdu, bool has_in_band,
   const struct dpif_backer_support *support,
@@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
 xbridge->ipfix = dpif_ipfix_ref(ipfix);
 }
 
+if (xbridge->lsample != lsample) {
+dpif_lsample_unref(xbridge->lsample);
+xbridge->lsample = dpif_lsample_ref(lsample);
+}
+
 if (xbridge->stp != stp) {
 stp_unref(xbridge->stp);
 xbridge->stp = stp_ref(stp);
@@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *xbridge)
 xlate_xbridge_set(new_xbridge,
   xbridge->dpif, xbridge->ml, xbridge->stp,
   xbridge->rstp, xbridge->ms, xbridge->mbridge,
-  xbridge->sflow, xbridge->ipfix, xbridge->netflow,
-  xbridge->forward_bpdu, xbridge->has_in_band,
-  &xbridge->support, xbridge->addr);
+  xbridge->sflow, xbridge->ipfix, xbridge->lsample,
+  xbridge->netflow, xbridge->forward_bpdu,
+  xbridge->has_in_band, &xbridge->support,
+