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
> > > 
> > 
> > 
> 


Reply via email to