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
