On Tue, Jul 09, 2024 at 11:45:35AM GMT, Eelco Chaudron wrote:
> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
>
> > Add as new column in the Flow_Sample_Collector_Set table named
> > "local_group_id" which enables this feature.
>
> Small nit below, if fixed add my ACK to the next revision.
>
> //Eelco
>
> > Signed-off-by: Adrian Moreno <amore...@redhat.com>
> > ---
> > NEWS | 4 ++
> > vswitchd/bridge.c | 78 +++++++++++++++++++++++++++++++++++---
> > vswitchd/vswitch.ovsschema | 9 ++++-
> > vswitchd/vswitch.xml | 40 +++++++++++++++++--
> > 4 files changed, 120 insertions(+), 11 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index e0359b759..15faf9fc3 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -16,6 +16,10 @@ Post-v3.3.0
> > per interface 'options:dpdk-lsc-interrupt' to 'false'.
> > - Python:
> > * Added custom transaction support to the Idl via add_op().
> > + - Local sampling is introduced. It reuses the OpenFlow sample action and
> > + allows samples to be emitted locally (instead of via IPFIX) in a
> > + datapath-specific manner. The Linux kernel datapath is the first to
> > + support this feature by using the new datapath "psample" action.
> >
> >
> > v3.3.0 - 16 Feb 2024
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index 95a65fcdc..b7db681f3 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -288,6 +288,7 @@ static void bridge_configure_mac_table(struct bridge *);
> > static void bridge_configure_mcast_snooping(struct bridge *);
> > static void bridge_configure_sflow(struct bridge *, int
> > *sflow_bridge_number);
> > static void bridge_configure_ipfix(struct bridge *);
> > +static void bridge_configure_lsample(struct bridge *);
> > static void bridge_configure_spanning_tree(struct bridge *);
> > static void bridge_configure_tables(struct bridge *);
> > static void bridge_configure_dp_desc(struct bridge *);
> > @@ -989,6 +990,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> > *ovs_cfg)
> > bridge_configure_netflow(br);
> > bridge_configure_sflow(br, &sflow_bridge_number);
> > bridge_configure_ipfix(br);
> > + bridge_configure_lsample(br);
> > bridge_configure_spanning_tree(br);
> > bridge_configure_tables(br);
> > bridge_configure_dp_desc(br);
> > @@ -1537,10 +1539,11 @@ ovsrec_ipfix_is_valid(const struct ovsrec_ipfix
> > *ipfix)
> > return ipfix && ipfix->n_targets > 0;
> > }
> >
> > -/* Returns whether a Flow_Sample_Collector_Set row is valid. */
> > +/* Returns whether a Flow_Sample_Collector_Set row contains a valid IPFIX
> > + * configuration. */
> > static bool
> > -ovsrec_fscs_is_valid(const struct ovsrec_flow_sample_collector_set *fscs,
> > - const struct bridge *br)
> > +ovsrec_fscs_is_valid_ipfix(const struct ovsrec_flow_sample_collector_set
> > *fscs,
> > + const struct bridge *br)
> > {
> > return ovsrec_ipfix_is_valid(fscs->ipfix) && fscs->bridge == br->cfg;
> > }
> > @@ -1558,7 +1561,7 @@ bridge_configure_ipfix(struct bridge *br)
> > const char *virtual_obs_id;
> >
> > OVSREC_FLOW_SAMPLE_COLLECTOR_SET_FOR_EACH(fe_cfg, idl) {
> > - if (ovsrec_fscs_is_valid(fe_cfg, br)) {
> > + if (ovsrec_fscs_is_valid_ipfix(fe_cfg, br)) {
> > n_fe_opts++;
> > }
> > }
> > @@ -1621,7 +1624,7 @@ bridge_configure_ipfix(struct bridge *br)
> > fe_opts = xcalloc(n_fe_opts, sizeof *fe_opts);
> > opts = fe_opts;
> > OVSREC_FLOW_SAMPLE_COLLECTOR_SET_FOR_EACH(fe_cfg, idl) {
> > - if (ovsrec_fscs_is_valid(fe_cfg, br)) {
> > + if (ovsrec_fscs_is_valid_ipfix(fe_cfg, br)) {
> > opts->collector_set_id = fe_cfg->id;
> > sset_init(&opts->targets);
> > sset_add_array(&opts->targets, fe_cfg->ipfix->targets,
> > @@ -1667,6 +1670,71 @@ bridge_configure_ipfix(struct bridge *br)
> > }
> > }
> >
> > +/* Returns whether a Flow_Sample_Collector_Set row contains a valid local
> > + * sampling configuration. */
> > +static bool
> > +ovsrec_fscs_is_valid_local(const struct ovsrec_flow_sample_collector_set
> > *fscs,
> > + const struct bridge *br)
> > +{
> > + return fscs->local_group_id && fscs->n_local_group_id == 1 &&
> > + fscs->bridge == br->cfg;
> > +}
> > +
> > +/* Set local sample configuration on 'br'. */
> > +static void
> > +bridge_configure_lsample(struct bridge *br)
> > +{
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > + const struct ovsrec_flow_sample_collector_set *fscs;
> > + struct ofproto_lsample_options *opts_array, *opts;
> > + size_t n_opts = 0;
> > + int ret;
> > +
> > + /* Iterate the Flow_Sample_Collector_Set table twice.
> > + * First to get the number of valid configuration entries, then to
> > process
> > + * each of them and build an array of options. */
> > + OVSREC_FLOW_SAMPLE_COLLECTOR_SET_FOR_EACH (fscs, idl) {
> > + if (ovsrec_fscs_is_valid_local(fscs, br)) {
> > + n_opts ++;
> > + }
> > + }
> > +
> > + if (n_opts == 0) {
> > + ofproto_set_local_sample(br->ofproto, NULL, 0);
> > + return;
> > + }
> > +
> > + opts_array = xcalloc(n_opts, sizeof *opts_array);
> > + opts = opts_array;
> > +
> > + OVSREC_FLOW_SAMPLE_COLLECTOR_SET_FOR_EACH (fscs, idl) {
> > + if (!ovsrec_fscs_is_valid_local(fscs, br)) {
> > + continue;
> > + }
> > + opts->collector_set_id = fscs->id;
> > + opts->group_id = *fscs->local_group_id;
> > + opts++;
> > + }
> > +
> > + 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));
> > + }
> > +
> > + if (n_opts > 0) {
> > + free(opts_array);
> > + }
> > +}
> > +
> > static void
> > port_configure_stp(const struct ofproto *ofproto, struct port *port,
> > struct ofproto_port_stp_settings *port_s,
> > diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> > index e2d5e2e85..95018d107 100644
> > --- a/vswitchd/vswitch.ovsschema
> > +++ b/vswitchd/vswitch.ovsschema
> > @@ -1,6 +1,6 @@
> > {"name": "Open_vSwitch",
> > - "version": "8.5.0",
> > - "cksum": "4040946650 27557",
> > + "version": "8.6.0",
> > + "cksum": "1543805939 27765",
> > "tables": {
> > "Open_vSwitch": {
> > "columns": {
> > @@ -562,6 +562,11 @@
> > "type": {"key": {"type": "uuid",
> > "refTable": "IPFIX"},
> > "min": 0, "max": 1}},
> > + "local_group_id": {
> > + "type": {"key": {"type": "integer",
> > + "minInteger": 0,
> > + "maxInteger": 4294967295},
> > + "min": 0, "max": 1}},
> > "external_ids": {
> > "type": {"key": "string", "value": "string",
> > "min": 0, "max": "unlimited"}}},
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index e3afb78a4..a162bfc5e 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -7008,10 +7008,37 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> > type=patch options:peer=p1 \
> >
> > <table name="Flow_Sample_Collector_Set">
> > <p>
> > - A set of IPFIX collectors of packet samples generated by OpenFlow
> > - <code>sample</code> actions. This table is used only for IPFIX
> > - flow-based sampling, not for per-bridge sampling (see the <ref
> > - table="IPFIX"/> table for a description of the two forms).
> > + A set of IPFIX or local sampling collectors of packet samples
> > generated
> > + by OpenFlow <code>sample</code> actions.
> > + </p>
> > +
> > + <p>
> > + If the column <code>ipfix</code> contains a reference to a
> > + valid IPFIX entry, samples will be emitted via IPFIX. This mechanism
> > + is known as flow-based IPFIX sampling, as opposed to bridge-based
> > + sampling (see the <ref table="IPFIX"/> table for a description of the
> > + two forms).
> > + </p>
> > +
> > + <p>
> > + If the column <code>local_group_id</code> contains an integer and the
> > + running datapath supports local sample emission, packets will be sent
> > + to some local sample collector. Samples will contain the group number
> > + specified by <code>local_group_id</code> which helps identify their
> > + source as well as a 64-bit cookie result from the concatenation of
> > the
> > + observation_domain_id an the observation_point_id in network order.
> > +
> > + The way the sample is emitted and made available for local collectors
> > + is datapath-specific.
> > +
> > + Currently only Linux kernel datapath supports local sampling which is
> > + implemented by sending the packet to the <code>psample</code> netlink
> > + multicast group.
> > + </p>
> > +
> > + <p>
> > + Note both <code>local_group_id</code> and <code>ipfix</code> can be
> > + configured simultaneously.
> > </p>
> >
> > <column name="id">
> > @@ -7030,6 +7057,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> > type=patch options:peer=p1 \
> > record per sampled packet to.
> > </column>
> >
> > + <column name="local_group_id"
> > + type='{"type": "integer", "minInteger": 0, "maxInteger":
> > 4294967295}'>
>
> nit: Other multi-line definitions have type= centered on name=.
>
Ack.
> > + Configuration of the sample group id to be used in local sampling.
> > + </column>
> > +
> > <group title="Common Columns">
> > The overall purpose of these columns is described under <code>Common
> > Columns</code> at the beginning of this document.
> > --
> > 2.45.2
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
