On Fri, Jun 24, 2016 at 02:54:23AM +0000, Wenyu Zhang wrote: > On 6/24/16, 2:18 AM, "Ben Pfaff" <b...@ovn.org> wrote: > > >On Mon, Jun 20, 2016 at 08:54:12PM -0700, Wenyu Zhang wrote: > >> In virtual network, users want more info about the virtual point to > >>observe the traffic. > >> It should be a string to provide clear info, not a simple interger ID. > >> > >> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string > >>configured by user. > >> Introduce an enterprise IPFIX entity "virtual_obs_id"(898) to export > >>the value. The entity is a > >> variable-length string. > >> > >> Signed-off-by: Wenyu Zhang <wen...@vmware.com>
... > >I think that the documentation that this adds is somewhat deceptive, > >because it implies that omitting the virtual_obs_id produces the same > >results as using an empty string. I believe that this is incorrect: if > >virtual_obs_id is omitted, then the virtual observation ID is not > >emitted at all, whereas if it is an empty string then it is emitted as > >an empty string. > Wenyu: I think that it seems no need to emit an empty string when the > ovirtual_obs_id is not set. > Do you think that emitting an empty string is a better solution? > > You are right. The document may mislead users. I may change it. The behavior in this case is not important to me, but I think that the documentation should be accurate. > >The documentation doesn't say what a "virtual observation ID" is. This > >is not a standard term, so users can't be expected to know. Please > >explain. In the end, I think that this is just an extra string that > >accompanies the flow records, so it may not be worthwhile to invent a > >special name for it like "virtual observation ID". > Wenyu: I want to introduce an entity to export the description of > observation info in virtual network. > Do you have any suggestion about the entity name ? This name is OK, as long as the documentation explains what it means. As is, it sounds like a virtual observation ID is something that the reader should understand. Please explain what it means in the documentation. > >In ipfix_put_data_set(), it is wasteful to use dp_packet_put_zeros() > >then immediately copy over all of the zeros. > > > >In ipfix_put_data_set(), this cast is not needed: > >+ data_virtual_obs_id->virtual_obs_len = (uint8_t)virtual_obs_len; > Wenyu: Due to the entity is variable-length, the length of it is necessary. The length is necessary, but it is not necessary to fill all of it with zeros and then copy over it. Use dp_packet_put_uninit() instead of dp_packet_put_zeros(). > I may put the byte into msg directly, instead of putting zeros and > overwrite it. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
