Thanks. I'll look through the resources and submit a new, correctly formatted patch soon. ________________________________ From: Ilya Maximets <i.maxim...@ovn.org> Sent: Friday, June 30, 2023 3:39 AM To: Sayali Naval (sanaval) <sana...@cisco.com>; d...@openvswitch.org <d...@openvswitch.org> Cc: i.maxim...@ovn.org <i.maxim...@ovn.org> Subject: Re: [ovs-dev] [PATCH v1] bridge ovs-vsctl Bridge IPFIX enable_input_sampling, enable_ouput_sampling fix unexpected values
On 6/21/23 20:43, Sayali Naval (sanaval) via dev wrote: > A gentle reminder to take a look at this patch. Hi. Sorry for a late reply. But this patch came in strangely formatted, so it wasn't recognized as a patch by any of our systems. The patch has extra spacing after every line that makes it not readable by automated tools and hence not applicable. Could you try re-sending it? Maybe re-configure your or use a different mail client for that, e.g. git send-email. There are some hints in the contribution guide: https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/ And some hints on mail client configuration from a kernel guide: https://static.lwn.net/kerneldoc/process/email-clients.html For example, here is how your patch looks like: https://mail.openvswitch.org/pipermail/ovs-dev/2023-May/405130.html And here is an example on how the patch formatting should look like: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405786.html Best regards, Ilya Maximets. > > Thanks, > Sayali Naval > ________________________________ > From: Sayali Naval (sanaval) > Sent: Wednesday, May 31, 2023 11:02 AM > To: d...@openvswitch.org <d...@openvswitch.org> > Subject: [ovs-dev] [PATCH v1] bridge ovs-vsctl Bridge IPFIX > enable_input_sampling, enable_ouput_sampling fix unexpected values > > > As per the Open vSwitch Manual > (http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt) the Bridge > IPFIX parameters can be passed as follows: > > > ovs-vsctl -- set Bridge br0 ipfix=@i \ > > -- --id=@i create IPFIX targets=\"192.168.0.34:4739\" obs_do‐ > > main_id=123 obs_point_id=456 cache_active_timeout=60 > > cache_max_flows=13 \ > > other_config:enable-input-sampling=false \ > > other_config:enable-output-sampling=false > > > where the default values are: > > enable_input_sampling: true > > enable_output_sampling: true > > > But in the existing code > https://github.com/openvswitch/ovs/blob/master/vswitchd/bridge.c#L1563-L1567, > these 2 parameters take up unexpected values in some scenarios. > > > > be_opts.enable_input_sampling = !smap_get_bool(&be_cfg->other_config, > > "enable-input-sampling", false); > > > be_opts.enable_output_sampling = !smap_get_bool(&be_cfg->other_config, > > "enable-output-sampling", > false); > > > Here, the function smap_get_bool is being used with a negation. > > > smap_get_bool is defined as below: > > (https://github.com/openvswitch/ovs/blob/master/lib/smap.c#L220-L232) > > > /* Gets the value associated with 'key' in 'smap' and converts it to a > boolean. > > * If 'key' is not in 'smap', or its value is neither "true" nor "false", > > * returns 'def'. */ > > bool > > smap_get_bool(const struct smap *smap, const char *key, bool def) > > { > > const char *value = smap_get_def(smap, key, ""); > > if (def) { > > return strcasecmp("false", value) != 0; > > } else { > > return !strcasecmp("true", value); > > } > > } > > > This returns expected values for the default case (since the above code will > negate “false” we get from smap_get bool function and return the value > “true”) but unexpected values for the case where the sampling value is passed > through the CLI. > > For example, if we pass "true" for other_config:enable-input-sampling in the > CLI, the above code will negate the “true” value we get from the smap_bool > function and return the value “false”. Same would be the case for > enable_output_sampling. > > > Signed-off-by: Sayali Naval <sana...@cisco.com> > > --- > > [bridge-ipfix-fix 277b3baae] Fix values for enable_input_sampling & > enable_ouput_sampling > > Date: Thu May 25 10:32:43 2023 -0700 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > > index f5dc59ad0..b972d55d0 100644 > > --- a/vswitchd/bridge.c > > +++ b/vswitchd/bridge.c > > @@ -1560,11 +1560,11 @@ bridge_configure_ipfix(struct bridge *br) > > be_opts.enable_tunnel_sampling = smap_get_bool(&be_cfg->other_config, > > "enable-tunnel-sampling", true); > > > > - be_opts.enable_input_sampling = !smap_get_bool(&be_cfg->other_config, > > - "enable-input-sampling", > false); > > + be_opts.enable_input_sampling = smap_get_bool(&be_cfg->other_config, > > + "enable-input-sampling", true); > > > > - be_opts.enable_output_sampling = > !smap_get_bool(&be_cfg->other_config, > > - "enable-output-sampling", > false); > > + be_opts.enable_output_sampling = smap_get_bool(&be_cfg->other_config, > > + "enable-output-sampling", > true); > > > > virtual_obs_id = smap_get(&be_cfg->other_config, "virtual_obs_id"); > > be_opts.virtual_obs_id = nullable_xstrdup(virtual_obs_id); > > -- > > > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
