Re: Encoding unknown values in the extended_switch structure

Hi Peter,

Not really an objection - rather a proposal for your consideration.

What about bitmapping fields contained in standard elements within,
say, the upper two bytes of the element tag?

typedef struct _SFLFlow_sample_element {
  struct _SFLFlow_sample_element *nxt;
  u_int32_t tag; /* SFLFlow_type_tag */ <-----
  u_int32_t length;
  SFLFlow_type flowType;
} SFLFlow_sample_element;

ie.

#define EX_SWITCH_SRC_VLAN 0x00010000
#define EX_SWITCH_SRC_PRIORITY 0x00020000
#define EX_SWITCH_DST_VLAN 0x00040000
#define EX_SWITCH_DST_PRIORITY 0x00080000

It would have the beauty of being backward compatible, meaning the
collector can just ignore it and not take benefit from it, and will
maintain intact the 32 bit field - which is not a requirement today,
for this specific case, but you might be less lucky tomorrow trying
to do the same trick elsewhere; so a more coherent approach.

Companies developing closed extensions and thus making use of the
upper bytes of the tag field, ie.

  /* enterprise = 4300 (inmon)...*/
  SFLFLOW_EX_PROCESS = (4300 << 12) + 3

might (or will need to) use a different way to signal some fields
are not populated within their elements.

The only con of this approach is that it would put an upper bound
to the number of fields that can be packed within an element, ie.
15 fields if using the upper 2 bytes (keeping out the '0x00000000'
value), but i see you have never been a big fan of flat structures
( :-) ) so i wouldn't see this as a true biggie. Quickly scanning
through the existing elements, all seem to fit this approach.

Cheers,
Paolo

On Fri, May 22, 2009 at 11:07:55AM -0700, Peter Phaal wrote:
> There currently isn't a satisfactory way to indicate that one or more of the
> fields in the extended_switch structure is unknown to the sFlow exporter.
>
> /* Extended Switch Data */
> /* opaque = flow_data; enterprise = 0; format = 1001 */
> /* Note: For untagged ingress ports, use the assigned vlan and priority
> of the port for the src_vlan and src_priority values.
> For untagged egress ports, use the values for dst_vlan and
> dst_priority that would have been placed in the 802.Q tag
> had the egress port been a tagged member of the VLAN instead
> of an untagged member. */
>
> struct extended_switch {
> unsigned int src_vlan; /* The 802.1Q VLAN id of incoming frame */
> unsigned int src_priority; /* The 802.1p priority of incoming frame */
> unsigned int dst_vlan; /* The 802.1Q VLAN id of outgoing frame */
> unsigned int dst_priority; /* The 802.1p priority of outgoing frame */
> }
>
> Since 0 is a valid value for both 802.1Q VLAN 802.1p priority it should not
> be used to encode the unknown value. Rather than omit the whole structure
> because one field isn't known, it makes sense to define an unambiguous value
> to indicate that a field is unknown.
>
> Fortunately these fields have relatively small maximum values, vlan <= 4096
> and priority <= 7, so an unambiguous choice for an unknown value would be
> 0xffffffff (Note: this same value that sFlow currently uses to indicate that
> a 32 bit counter value is unknown). If there are no objections, I think it
> would make sense to include a note to this effect on Specifications page on
> sFlow.org.
>
> Peter
Received on Sat May 23 04:59:37 2009