Thanks. I agree that we should remove IPFIX and sample action support.
I will send v4 for review.
Regards,
William
On Fri, Jul 22, 2016 at 2:38 PM, Ben Pfaff <b...@ovn.org> wrote:
> It sounds like snaplen is not very useful in IPFIX. If so, then I'd
> support removing the IPFIX and sample action support for it.
>
> If you agree, will you send a v4? Otherwise, let me know and I'll apply
> v3.
>
> Here's a change I'd like to fold into v3, to better allow for future
> extensions. It matches the way that we treat padding fields in most
> actions these days.
>
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index a3a14b0..cd528a4 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -4779,7 +4779,7 @@ struct nx_action_sample2 {
> ovs_be32 obs_point_id; /* ID of sampling observation point. */
> ovs_be16 sampling_port; /* Sampling port. */
> ovs_be16 snaplen; /* Max sampled packet size in byte. */
> - uint8_t pad[4]; /* Pad to a multiple of 8 bytes */
> + uint8_t zeros[4]; /* Pad to a multiple of 8 bytes */
> };
> OFP_ASSERT(sizeof(struct nx_action_sample2) == 32);
>
> @@ -4812,6 +4812,10 @@ decode_NXAST_RAW_SAMPLE2(const struct
> nx_action_sample2 *nas,
> enum ofp_version ofp_version OVS_UNUSED,
> struct ofpbuf *out)
> {
> + if (!is_all_zeros(nas->zeros, sizeof nas->zeros)) {
> + return OFPERR_NXBRC_MUST_BE_ZERO;
> + }
> +
> struct ofpact_sample *sample;
>
> sample = ofpact_put_SAMPLE(out);
>
>
> Thanks,
>
> Ben.
>
> On Sun, Jul 10, 2016 at 09:24:45PM -0700, William Tu wrote:
>> Hi Daniel,
>>
>> Thanks for reviewing the patch.
>> Indeed, the way sFlow sets up the datapath does not require the
>> OpenFlow sample action, and changing OVSDB/compose_sample_action() is
>> sufficient to program the datapath sample action for current sFlow use
>> case. So if IPFIX does not use 'snaplen', or there is no future use
>> case, then I could remove the snaplen in "struct nx_action_sample2".
>>
>> Regards,
>> William
>>
>> On Sun, Jul 10, 2016 at 7:46 PM, Daniel Ye <dani...@vmware.com> wrote:
>> > Hi William,
>> >
>> >
>> >
>> > I saw that you submitted the patch below. It’s about adding ‘snaplen’ for
>> > sample action and sFlow.
>> >
>> > https://patchwork.ozlabs.org/patch/645013/
>> >
>> >
>> >
>> > I’m still confused whether sFlow uses the openflow sample action. In my
>> > opinion, sFlow only uses the datapath
>> >
>> > sample action, not the sample action in userworld. Actually, there are two
>> > kinds of IPFIX, one is bridge level IPFIX and the
>> >
>> > other is flow-based IPFIX. Bridge level IPFIX doesn’t use openflow sample
>> > action, it only uses the datapath sample action. I think
>> >
>> > sFlow does the same as bridge level IPFIX. For bridge level IPFIX and
>> > sFlow,
>> > all the parameters are stored in OVSDB.
>> >
>> >
>> >
>> > Actually, “nx_action_sample2” is only used by flow-based IPFIX now. If you
>> > add a ‘snaplen’ field in it, I don’t think this field
>> >
>> > will be used by sFlow. Maybe you only need to change
>> > “compose_sample_action()” and put ‘snaplen’ in OVSDB??
>> >
>> >
>> >
>> > For all the alias, does any sFlow experts come to make sure of this?
>> >
>> >
>> >
>> > Thanks,
>> >
>> > Daniel Benli Ye
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
