On Mon, 2021-03-22 at 09:00 +0000, Stuart Henderson wrote: > On 2021/03/22 00:20, Martijn van Duren wrote: > > There's two things I'd like to know before going further with this: > > > > 1) according to RFC2578 section 3.6: > > (1) registration: the definition of a particular item is registered as > > a particular OBJECT IDENTIFIER value, and associated with a > > particular descriptor. After such a registration, the semantics > > thereby associated with the value are not allowed to change, the > > OBJECT IDENTIFIER can not be used for any other registration, and > > the descriptor can not be changed nor associated with any other > > registration. The following macros result in a registration: > > > > OBJECT-TYPE, MODULE-IDENTITY, NOTIFICATION-TYPE, OBJECT-GROUP, > > OBJECT-IDENTITY, NOTIFICATION-GROUP, MODULE-COMPLIANCE, > > AGENT-CAPABILITIES. > > > > Unless I'm misreading this section this would require a registration of > > a new object to strictly comply with SMIv2. Is this change minor enough > > to allow it? > > I suppose it depends what they mean by semantics. The type doesn't change, > it's still an OCTET STRING, the data don't change, the only difference is > the DISPLAY-HINT. I think in this case changing oid will cause more problems > (breaking config for anyone aleeady monitoring this) than it solves. > > In terms of side-effects, the main one is that software indexing data > storage by the name in ifDescr after display-formatting may go from hex > bytes to readable strongs (the case with snmp_exporter->prometheus). > But I think most cases where people actually monitor OpenBSD MIBs are > more likely to be based on with net-snmp or possovly snmp(1) both of > which just interpret these as STRING anyway. > > And in cases where this does change something it's easier to adapt to a > display format change than an oid change. > > > 2) Afaik any byte sequence is technically allowed for these in pf.conf. > > (at least for pfLabelName and pfTableName I manage to make UTF-8 work), > > so changing this would technically mean that we wouldn't be able to > > export these tables and labels anymore. Note that RFC2579 section 3.1 > > bullet 3 of octet-format doesn't specify how to handle invalid ascii or > > UTF-8. In snmp(1) we went with the replacement character, but how any > > other pieces of software would handle that, I don't know. Is this > > change safe enough in all situations? > > > > Pending the answers, from my limited knowledge of pf and the MIB I > > guess the impact on pfLogIfName and pfIfDescr is limited enough since > > they show the actual interface name, which is always ascii anyway. > > For pfTableName and pfLabelName it might be an idea to register a new > > object which returns something like a best effort UTF-8 presentation > > of the actual name and go with SnmpAdminString. > > Good catch about UTF-8, so these should be SnmpAdminString, I think the > same applies about changing oid though.
For the second one I was talking about addition instead of changing, since technically any non-NUL byte sequence can be entered. But I can see how that's far from the intention and merely an implementation detail. So I think pfLogIfNamea and pfIfDescr can be DisplayString and pfTableName and pfLabelName should be snmpAdminstring (or have an snmpAdminString additional column). With that and the appropriate corresponding change to REVISION/DESCRIPTION OK martijn@. > > > > On Sat, 2021-03-20 at 14:29 +0000, Stuart Henderson wrote: > > > DisplayString seems a better type for pfIfDescr and some of the other > > > objects; this improves the display format in Prometheus snmp_exporter > > > which uses hex values for OCTET STRING. > > > > > > OK? > > > > > > > > > Index: OPENBSD-PF-MIB.txt > > > =================================================================== > > > RCS file: /cvs/src/share/snmp/OPENBSD-PF-MIB.txt,v > > > retrieving revision 1.6 > > > diff -u -p -r1.6 OPENBSD-PF-MIB.txt > > > --- OPENBSD-PF-MIB.txt 19 Jun 2018 10:08:45 -0000 1.6 > > > +++ OPENBSD-PF-MIB.txt 20 Mar 2021 14:24:11 -0000 > > > @@ -23,7 +23,7 @@ IMPORTS > > > TimeTicks, enterprises > > > FROM SNMPv2-SMI > > > > > > - TruthValue > > > + TruthValue, DisplayString > > > FROM SNMPv2-TC > > > > > > openBSD > > > @@ -33,7 +33,7 @@ IMPORTS > > > FROM SNMPv2-CONF; > > > > > > pfMIBObjects MODULE-IDENTITY > > > - LAST-UPDATED "201506091728Z" > > > + LAST-UPDATED "202103201421Z" > > > ORGANIZATION "OpenBSD" > > > CONTACT-INFO " > > > Author: Joel Knight > > > @@ -43,6 +43,8 @@ pfMIBObjects MODULE-IDENTITY > > > DESCRIPTION "The MIB module for gathering information from > > > OpenBSD's packet filter. > > > " > > > + REVISION "202103201421Z" > > > + DESCRIPTION "Use DisplayString not OCTET STRING where appropriate" > > > REVISION "201506091728Z" > > > DESCRIPTION "Add separate counter for failed 'route-to' applications" > > > REVISION "201308310446Z" > > > @@ -300,7 +302,7 @@ pfStateRemovals OBJECT-TYPE > > > -- pfLogInterface > > > > > > pfLogIfName OBJECT-TYPE > > > - SYNTAX OCTET STRING > > > + SYNTAX DisplayString > > > MAX-ACCESS read-only > > > STATUS current > > > DESCRIPTION > > > @@ -683,7 +685,7 @@ pfIfEntry OBJECT-TYPE > > > PfIfEntry ::= > > > SEQUENCE { > > > pfIfIndex Integer32, > > > - pfIfDescr OCTET STRING, > > > + pfIfDescr DisplayString, > > > pfIfType INTEGER, > > > pfIfRefs Unsigned32, > > > pfIfRules Unsigned32, > > > @@ -719,7 +721,7 @@ pfIfIndex OBJECT-TYPE > > > ::= { pfIfEntry 1 } > > > > > > pfIfDescr OBJECT-TYPE > > > - SYNTAX OCTET STRING > > > + SYNTAX DisplayString > > > MAX-ACCESS read-only > > > STATUS current > > > DESCRIPTION > > > @@ -913,7 +915,7 @@ pfTblEntry OBJECT-TYPE > > > TblEntry ::= > > > SEQUENCE { > > > pfTblIndex Integer32, > > > - pfTblName OCTET STRING, > > > + pfTblName DisplayString, > > > pfTblAddresses Integer32, > > > pfTblAnchorRefs Integer32, > > > pfTblRuleRefs Integer32, > > > @@ -947,7 +949,7 @@ pfTblIndex OBJECT-TYPE > > > ::= { pfTblEntry 1 } > > > > > > pfTblName OBJECT-TYPE > > > - SYNTAX OCTET STRING > > > + SYNTAX DisplayString > > > MAX-ACCESS read-only > > > STATUS current > > > DESCRIPTION > > > @@ -1363,7 +1365,7 @@ pfLabelEntry OBJECT-TYPE > > > PfLabelEntry ::= > > > SEQUENCE { > > > pfLabelIndex Integer32, > > > - pfLabelName OCTET STRING, > > > + pfLabelName DisplayString, > > > pfLabelEvals Counter64, > > > pfLabelPkts Counter64, > > > pfLabelBytes Counter64, > > > @@ -1383,7 +1385,7 @@ pfLabelIndex OBJECT-TYPE > > > ::= { pfLabelEntry 1 } > > > > > > pfLabelName OBJECT-TYPE > > > - SYNTAX OCTET STRING > > > + SYNTAX DisplayString > > > MAX-ACCESS read-only > > > STATUS current > > > DESCRIPTION > > > > > > > >