Re: [nox-dev] Getting Actions From Events in Python
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 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(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(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?
>>
>> Than
Re: [nox-dev] Getting Actions From Events in Python
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 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(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(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.o
Re: [nox-dev] Getting Actions From Events in Python
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(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(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
Re: [nox-dev] Getting Actions From Events in Python
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(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(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
Re: [nox-dev] Getting Actions From Events in Python
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(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(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
Re: [nox-dev] Getting Actions From Events in Python
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
[nox-dev] Getting Actions From Events in Python
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
