Derek and I fixed this up quite a while ago. It's one of the commits that got lumped together and doesn't have a good commit message, but it's been in destiny for months now. (One day I may actually fix the commit log for it.)
-- Murphy On Oct 5, 2011, at 2:39 PM, Christian Esteve Rothenberg wrote: > Hi Derek, > > did you succeed in implementing the to_python() for the action list in > flow_stats? > > Does someone have a patch for this issue? > > Thanks, > Christian > > On Fri, Jan 28, 2011 at 11:51, James "Murphy" McCauley <jam...@nau.edu> wrote: >> Yeah, I think this looks like the right direction. I probably wouldn't >> have bothered writing a to_python for ofp_action_header since we know >> it's not used anywhere else, and would have just handled it in the one >> for Flow_stats itself, but this is not really here nor there. >> >> This is a fine use for reinterpret_cast (except for the last one which >> you probably noticed needs a little work ;) ). >> >> I don't think that the length of the actions are checked anywhere (I >> could be wrong on this), so it might be nice to make sure that the >> length field of an action actually matches the size of the struct you're >> casting it to. Really we should handle this gracefully (e.g., log it >> and don't Pythonize it), but personally, I'd be okay with just doing an >> assert, since I would not expect to ever actually see it, but an assert >> is better than a segfault if it does pop up. :) Of course, there's also >> the argument that we should be validating them earlier. Whatever. Up >> to you what (if anything) you want to do about it in any case. >> >> -- Murphy >> >> On Fri, 2011-01-28 at 16:50 +0900, Derek Cormier wrote: >>> Without minding the typos, that is... >>> >>> On 01/28/2011 04:46 PM, Derek Cormier wrote: >>>> Ok I'll give it a try and submit a patch when I'm finished. Could you >>>> please tell me if the following is >>>> the right approach? >>>> >>>> template <> >>>> PyObject* >>>> to_python(const ofp_action_header &a) >>>> { >>>> PyObject* dict = PyDict_New(); >>>> if (!dict) { >>>> return 0; >>>> } >>>> >>>> uint16_t type = ntohs(a.type); >>>> pyglue_setdict_string(dict, "type", to_python(type)); >>>> pyglue_setdict_string(dict, "length", to_python(ntohs(a.len)); >>>> >>>> /* depending on the action type, cast to the appropriate >>>> * action struct to get its fields. */ >>>> if (type == OFPAT_OUTPUT) { >>>> const ofp_action_output& ao = >>>> reinterpret_cast<ofp_action_output&>(a); >>>> uint16_t port = ntohs(ao.port); >>>> >>>> pyglue_setdict_string(dict, "port", to_python(port)); >>>> >>>> /* max_len only has meaning when the destination port is the >>>> controller */ >>>> if (port == OFPP_CONTROLLER) { >>>> pyglue_setdict_string(dict, "max_len", >>>> to_python(ntohs(ao.max_len))); >>>> } >>>> } >>>> else if (type == OFPAT_STRIP_VLAN) { >>>> /* nothing to set, no struct beyond the header */ >>>> } >>>> else if (type == OFPAT_SET_VLAN_VID) { >>>> const ofp_action_vlan_vid av = >>>> reinterpret_cast<ofp_action_output&>(a); >>>> .... >>>> >>>> Is this the proper use of reinterpret cast? I've never had to use it >>>> before... >>>> >>>> -Derek >>>> >>>> On 01/28/2011 03:20 PM, James "Murphy" McCauley wrote: >>>>> I believe the issue is that with the to_python() for ofp_flow_stats&, we >>>>> have no idea if the actions actually follow the ofp_flow_stats structure >>>>> since, for example, someone could have just made an ofp_flow_stats >>>>> struct and tried to pythonize it. I am not sure if this ever actually >>>>> happens, but whatever. It's not provably a safe thing to do. However, >>>>> the Flow_stats struct actually explicitly has the actions wrapped up >>>>> into a vector, so it IS possible to safely pull them out of that. It >>>>> just hasn't been done. >>>>> >>>>> The to_python() for Flow_stats should: Step one, convert the fields from >>>>> the ofp_flow_stats struct itself into "dict", which is done by just >>>>> calling the to_python() for ofp_flow_stats. Step two, unpack v_actions >>>>> from the Flow_stats struct into a list of dicts and throw it into "dict" >>>>> too. Except instead of step two, we have /* XXX actions */. :) You >>>>> should be able to just go ahead and implement it there. >>>>> >>>>> -- Murphy >>>>> >>>>> On Fri, 2011-01-28 at 14:50 +0900, Derek Cormier wrote: >>>>>> Hello, >>>>>> >>>>>> I was looking at pyglue.cc and it looks like actions are never included >>>>>> in python events. >>>>>> >>>>>> A comment says to use Flow_stats, but flow stats contains a vector >>>>>> ofp_action_header's. Since actions are variable-length, won't this cut >>>>>> off data when the action is longer than the header? (For example, >>>>>> ofp_action_dl_addr). >>>>>> >>>>>> So, is there any way to get actions in events like flow stats in? If >>>>>> not, how could we go about implementing this? >>>>>> >>>>>> Thanks, >>>>>> Derek >>>>>> >>>>>> _______________________________________________ >>>>>> nox-dev mailing list >>>>>> nox-dev@noxrepo.org >>>>>> http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >>>>> >>>>> >>>> >>> >> >> >> >> _______________________________________________ >> nox-dev mailing list >> nox-dev@noxrepo.org >> http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >> > > > > -- > Christian _______________________________________________ nox-dev mailing list nox-dev@noxrepo.org http://noxrepo.org/mailman/listinfo/nox-dev