[kea-dev] Strange behaviour with Option 53 - what am I missing?

[Dave Cole]

I was hoping to be able to minimise the amount of extra work done by the binding, so eliminating unnecessary object allocation / destruction would be a good thing.  I recently had a play with exposing the shared_ptr use_count to confirm a behaviour I was seeking.  It turns out that the shared_ptr works very nicely with Python wrapping.  I would like to be able to create constant options once at module load time and then poke them into response packets as required.  This will completely eliminate heap activity for those objects.


All of my Python classes are just simple proxies for the wrapped Kea class.  They just act as vehicles for delivering the Kea object to Kea code and are never retained by any object reference from Kea.  The only exception to this rule is the CalloutHandle setContext() method where the roles are reversed :-).


>>> from kea import *
>>> o = Option(1).setUint8(3)
>>> o.use_count
1
>>> p = Pkt4(DHCPDISCOVER, 1)
>>> p.addOption(o)
>>> o.use_count
2
>>> p = None
>>> o.use_count
1
>>> p = Pkt4(DHCPDISCOVER, 1)
>>> p.addOption(o)
>>> o.use_count
2
>>> p.pack()
>>> o.use_count
2
>>> p = None
>>> o.use_count
1
>>> p = Pkt4(DHCPDISCOVER, 1)
>>> p.addOption(o)
>>> q = Pkt4(DHCPDISCOVER, 1)
>>> q.addOption(o)
>>> o.use_count
3
>>> p = q = None
>>> o.use_count
1
>>> import sys
>>> sys.getrefcount(o)
2
>>> p = Pkt4(DHCPDISCOVER, 1)
>>> p.addOption(o)
>>> sys.getrefcount(o)
2
>>> sys.getrefcount(Option(1))
1



________________________________
From: Francis Dupont <fdupont at isc.org>
Sent: Tuesday, 30 January 2018 7:34:07 PM
To: Dave Cole
Cc: kea-dev at lists.isc.org
Subject: Re: [kea-dev] Strange behaviour with Option 53 - what am I missing?

Dave Cole writes:
> >>> from kea import *
> >>> p =3D Pkt4(DHCPDISCOVER, 0)
> >>> o =3D p.getOptions()[DHO_DHCP_MESSAGE_TYPE]
> >>> o.getUint8()
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> TypeError: Attempt to read uint8 from option 53 that has size 0
> >>> o.getString()
> '\x01'
>
> The implementation of getUint8 is:
>
> static PyObject *
> Option_getUint8(OptionObj *self, PyObject *args) {
>     try {
>         return (PyInt_FromLong(self->option_ptr->getUint8()));
>     } catch (const exception &e) {
>         return (raisePythonError(PyExc_TypeError, e.what()));
>     }
> }

=> you don't use the right method: the option 53 dhcp-message-type is
decoded as an OptionInt<uint8_t> not as an Option so its content is
available by getValue().
 If you look at the option_int.h file you can see the constructor
with begin/end content iterators does not pass them to the super class
but only to unpack.
 You have the same for most of derived classes from Option, I believe
the idea is to save memory: as the content is decoded there is no need
to save it also in the super class object.
 Of course this means you have to use a dynamic cast to the derived class
and specialized methods. Or you can repack the option in wire format
using toBinary() and works on the result.

> Whereas the getString is:
>
> static PyObject *
> Option_getString(OptionObj *self, PyObject *args) {
>     try {
>         vector<uint8_t> value = self->option_ptr->toBinary();
>         return (PyString_FromStringAndSize((const char *) &value[0], value.=
> size()));
>     } catch (const exception &e) {
>         return (raisePythonError(PyExc_TypeError, e.what()));
>     }
> }
>
> So when I step through the code with gdb it appears that DHO_DHCP_MESSAGE_T=
> YPE is an instance of OptionInt which does not use the data_ member of Opti=
> on, instead it has another member value_ of the type defined in the templat=
> e argument.
>
> Is it a deliberate choice in the code to require application code to just k=
> now that getUint8 does not work for OptionInt (which subclasses Option)?

=> it is.

> Someone with more of a clue about c++ than me might be able to tell me an e=
> asy (and efficient) way I can make my code notice an OptionInt and internal=
> ly use getValue() rather than getUint8().  Would a dynamic_pointer_cast<Opt=
> ionUint8>(self->option_ptr) be the way to go?

=> you should get the definition from either std_option_defs.h or
runtime libdhcp++ structures and work with it. Or simply repack the
option with toBinary(). In fact it depends on the option format: if it
is simple repacking is easier, if it is complex the OptionCustom provides
tools which makes redecoding useless.

Thanks

Francis Dupont <fdupont at isc.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.isc.org/pipermail/kea-dev/attachments/20180130/372d59d1/attachment.html>