[ovs-discuss] new experimenter action.

[Ben Pfaff]

Thanks for writing this up.

If you wanted to expand on this a bit then I'd be happy to see it posted
as a blog entry in some appropriate place.  Then it would be easier to
point people looking for guidance to it.

On Fri, May 08, 2015 at 03:47:17PM -0500, David Evans wrote:
> Hi Ben,
> Thank you for your guidance so far  - here is my accumulated knowledge
> back into the discussion.
> 
> As I understand it - and this could be the beginning of a HOWTO of sorts
> for adding a custom packet action - munger/modifier/handler
> 
> openvswitch.h  
>     add OVS_ACTION_ATTR_FOO_MY_PACKET and describe arguments if required
> packets.c & packets.h  - my fn & pro to goes here.
>     void foo_my_packet(struct dp_packet *packet)… Etc.
> actions.c 
>     do_execute_actions() requires a case to handle
> OVS_ACTION_ATTR_FOO_MY_PACKET
> 
> 
> Open flow side
> ofp-actions.c
>     Add enum value to ofp_raw_action_type for experimenter
>     DE_RAW_FOO_MY_PACKET (with comment formatted as required for possible
> additional data & id for experimenter)
>     Add Format / decode / encode / parse functions
>     ―nb for encode, I couldn’t get ofpact_put_FOO_MY_PACKET(out) to behave
> and 
>      I’m not sure why.
>      But ofpact_put_raw(out, ofp_version, DE_RAW_FOO_MY_PACKET, 0); did
> the trick
> 
> 
> ofp-actions.h
>     Add to #define OFPACTS and include matching data structure if required.
> build-aux/extract-ofp-actions  (the python magic that makes .inc files)
>     Add my experimenter vendor id
> ofproto-dpif-xlate.c
>     static void compose_foo_my_packet_action(struct xlate_ctx *ctx){
>         nl_msg_put_u32(ctx->xout->odp_actions,
> OVS_ACTION_ATTR_FOO_MY_PACKET,0);
>         if(ctx->xin->packet) foo_my_packet(ctx->xin->packet);
>     }
> 
>     And 
>     To do_xlate_actions() add
>     case OFPACT_FOO_MY_PACKET:
>                 compose_foo_my_packet_action(...
> odp-util.c
>     format_odp_action() requires a case as does odp_action_len() entry for
> the size of required param if any.
> flow-netlink.c
>     __ovs_nla_copy_actions requires an action_lens array entry for the
> size of required param if any.
> 
> dpif-netdev.c
> 
>     May require dp_execute_cb() OVS_NOT_REACHED case.
> dpif.c
>     May require dpif_execute_helper_cb OVS_NOT_REACHED case
> 
> odp-execute.c 
>     add to odp_execute_actions a case to handle
> OVS_ACTION_ATTR_FOO_MY_PACKET
> 
> Compile and catch bugs - repeat.
> 
> Thanks again!
> 
> 
> On 5/6/15, 5:58 PM, "David Evans" <davidjoshuaevans at gmail.com> wrote:
> 
> >Thank you.
> >That was helpful.
> >I found that using the default ofpact_put* failed. And I used
> >ofpact_put_raw() instead. I don¹t know why. But I kept getting illegal
> >length not %8 from ovs-ofctl.
> >
> >Now I see some compose_* actions in the ofproto-dpif-xlate.c
> >But I don¹t see how anything turns from a OFPACT_* into an
> >OVS_ACTION_ATTR_* ?
> >Where does this part of the translation from openflow action to datapath
> >action happen.
> >I don¹t see any tables, or comparison items that would lead me to be able
> >to match my OVS_ACTION_ATTR_FOO_MY_PKTS against my new ofp_raw_action_type
> >Value DE_RAW_FOO_MY_PKTS action.
> >
> >I think do_xlate_actions is supposed to build a ctx for thisŠ but I am at
> >a loss to understand how I do the above.
> >
> >Regards,
> >Dave.
> >
> >
> >On 5/6/15, 12:40 PM, "Ben Pfaff" <blp at nicira.com> wrote:
> >
> >>On Wed, May 06, 2015 at 08:00:07AM -0500, David Evans wrote:
> >>> I have gotten a little further.
> >>> My new flow action registers ok in ovs, but when i use ovs-ofctl to
> >>>dump
> >>> the flows, the reply message parser in ovs-ofctl bails out trying to
> >>>read
> >>> the instruction/actions from the message saying that they're an invalid
> >>> length.
> >>> Do i need a vendor_id specific stats reply or something like that to
> >>>fix
> >>> this?
> >>
> >>It seems likely that the encode_*() function that you wrote does not
> >>properly encode the new action.  Does it use the generated
> >>ofpact_put_*() function to encode the action?  I would also check the
> >>hexdump for the action to make sure that it has the expected format.
> >>
> >>You do need a "vendor extension" or "experimenter" action but the
> >>generated ofpact_put_*() function should take care of that for you.
> >
> >
> 
>