Re: [IPFIX] Initial review of draft-ietf-ipfix-mib-variable-export-03

Hi Brian,

Thanks for the review see replies inline on some of your points:

On 01/11/2013 03:47, Brian Trammell wrote:
> Greetings, all,
>
> I've had a quick look at the present (-03) revision of the MIB Variable
> Export draft. The document still needs at least one revision before it's
> complete, as noted in its Open Issues section. I'll do a full review
> after the open issues are handled.
Agreed this document is still a work in progress.

>
> But the fundamental change in this revision -- moving to an Options
> Template based mechanism for binding OIDs to templates -- looks good,
> and seems to be the right way to move this work forward.
>
> I should note I still have no real understanding of how indexing works
> in SNMP, despite having made an effort a couple of times now, and this
> document makes it seem even more complicated than I thought it was. That
> having been said, after having how often are indexes in SNMP used to
> bind values to things we'd think of as flow keys in IPFIX? Does it make
> sense to try to repurpose flowKeyIndicator here? This seems to be the
> case in Section 6.4, but maybe I'm latching on to irrelevant details in
> the example.
The complexity that still remains is an attempt to allow existing keys
in a data record to be used to index into the columns of data.

The other option is to just export an extra 'string' in the data record
if there are no key fields that can be used. That is what mibIndexString is
for.

I'll need to look into using a similar method to flowKeyIndicator. And see
if there is a clearer way to do the indexing/explain it.

Using a bit flag to mark the fields indexing a mibObjectValue would also
be more efficient than giving a list of indexes into a template. What you wouldn't
be able to specify is the ordering that might be different than the order
of the fields in the template.

>
> I have quite a few issues with the specifics of the mechanism and the
> document itself. Since the details of the mechanism are described by the
> IEs used in the options templates, I started in the IANA Considerations
> section:
>
> (1) Section 11.1 doesn't make any sense and should be removed. Unless
> I've missed something fundamental, nothing else in the document suggests
> that new Abstract Data Types are necessary for this mechanism to work.
> If I have missed something fundamental, the mechanism should be
> redefined such than new Abstract Data Types are not necessary.

I don't think we need to have new Abstract Data Types to make the mechanism work.

We would only need new ADTs if we are actually adding extra snmp types.

>
> (2) Usage of Data Type Semantics throughout section 11.2 is not
> consistent with RFC 7012; non-numeric data types do not take Data Type
> Semantics, as the semantics are inherent to the Abstract Data Type.
>
Noted.

> (3) Clarifying and SNMP-naif question on 11.2.2 mibObjectIdentifier: it
> seems from the examples that the intention is that the OID will be
> encoded in the document as a Unicode string of digits and "."
> characters? Is this the way OIDs are encoded in SNMP? The encoding of
> fundamentally binary data in strings is not very IPFIXish (7013 section
> 4.2 para 6), and I kind of expected these to be encoded as octetarrays
> of unsigned integers of a specified size, or something similar. However,
> if encoding-as-string is how things are done on the wire in SNMP, it's
> the right thing to do here too (7013 section 4.5). The same comment
> applies to 11.2.4.
>

This is carried over from the previous versions of the draft so I'm not sure about the
background on the choice.

I agree that encoding the mibObjectIdentifier in the same format as SNMP might
be ideal as we would have a complete definition in another RFC.

Sending the OID as a string is 'simpler' but potentially wasteful. On the other
hand a lot of the sub-identifiers are short and would be more efficient as a single
character than a 32bit number.

Note that this string is only being sent once for each mib field. The records using
mib objects are not exporting any extra strings.

> (4) In 11.2.3 mibIndexList: the underlying data type of the elements of
> the list encoded in the octetArray is missing from the description - are
> these unsigned8s or unsigned16s? The encoding of internal structure
> which doesn't come from some external protocol in an octetArray violates
> 7013 section 4.2. It seems like what you really want here is an
> Information Element that is an instance of a basicList ADT with a
> constraint in the description that states that only
> informationElementIndex IEs are allowed as content.

I did consider writing this using the basicList structured data that might be
clearer. We also discussed that if the type of the fields and the elements never changes it
seems wasteful in the simple case of a single index.

Defining a single mibIndex field and a basicList mibIndexList field might be
the best compromise.

I haven't fully groked '7013 section 4.2' - I'll go and reread it.

>
> (5) Also on 11.2.3 and 11.2.4, see comment above on using
> flowKeyIndicator instead, if indeed that suggestion makes any sense.
>
We might still want the definition in mibIndexString 11.2.4 as that allows indexing
in a different manner.

> (6) In 11.2.7 mibObjectBaseSyntax, I think the question "should there be
> 11 different IEs, one of each of the above types, rather than a single
> mibObjectValue?" is definitely worth considering, as it would allow
> faster handling at CPs based on the base syntax without having to decode
> the base syntax from the options template.

That was my thinking by requiring the basesyntax field - and having that
information in the data template is probably worth having the extra fields.

Also aside from having to decode the options template the cp needs to have
access to the correct mib to work out the base data type for the field. Depending
on the implementation the correct mib might be missing or require a network lookup.

While the OID/ ObjectIdentifier provides enough information in theory to understand
the field it might take some 'work' - providing the base syntax or encoding the
base type in the IE is a nice compromise - the CP can store/display the values nicely
and always lookup the MIB later.

> Not being a SNMP geek, I don't understand why you need 11 here.

I included all 11 base 'types' in the baseSyntax as I didn't understand the
nuiances of the differences. It also made it simpler when mapping from the
definitions of mib objects.

> Reduced length encoding and  mappings to IPFIX ADTs and semantics suggest
> the following seven mappings to IPFIX IEs would suffice:
>
> OCTET STRING and Opaque -> octetArray
> INTEGER and Integer32 -> signed64 (with RLE) / quantity
> Counter32 and Counter64 -> signed64 (with RLE) / totalCounter
> Gauge32 and Unsigned32 -> unsigned64 (with RLE) / quantity
> IpAddress -> ipv4Address (side question: how do you do V6 with SNMP?)
> TimeTicks -> dateTimeSeconds (for IPFIX Epoch)
> OBJECT IDENTIFIER -> see 11.2.2
>
> There are some mismatches between base syntaxes and IPFIX data types
> which it might be necessary to address.

Yes I don't think we want to lose any type information when exporting mib
data types via ipfix. I don't see any problem with using RLE though.

>  If keeping SNMP Counter > semantics is important (it seems like it might be),
>  it might make sense to add an snmpCounter Data Type Semantics. The minimum and maximum
> values for a gauge can be bound to the gauge with
> informationElementRangeBegin and informationElementRangeEnd from RFC 5610.
>
> Doing this would obviate the need for the mibObjectBaseSyntax, no?

Yes the cp would know the basic data for the field even without having access
to the mib so it wouldn't need to be in the mibOption. The only required field
would be the mibObjectIdentifier, which is nicer.

>
> (7) In 11.2.10 mibContextIdentifier: is a context identifier similar to
> an object identifier? Then it should be defined similar to 11.2.2 -- it
> doesn't need a new ADT.
>
> Editorial commentary (not exhaustive -- I'll wait 'til the open issues
> are fixed before doing a full review):
>
> (8) Section 5.1.2 "Minimum Required MIB Object Fields" is hard to
> visually scan -- I had to look over it several times before I understood
> that the things in the list are _optional_ while the things buried in
> the first paragraph are required. Suggest using list formatting for all.

Agree, if we can simplify to one required field this can all be made clearer as well.


Thanks,
colin

>
> Best regards,
>
> Brian
>
>
>
> _______________________________________________
> IPFIX mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/ipfix
>
_______________________________________________
IPFIX mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ipfix