Subject is incorrect, missing v2. Please use this instead: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341658.html
> -----Original Message----- > From: Zoltán Balogh > Sent: Wednesday, December 06, 2017 11:55 AM > To: [email protected] > Cc: Zoltán Balogh <[email protected]>; Sugesh Chandran > <[email protected]>; Ben Pfaff <[email protected]> > Subject: [PATCH 2/2] xlate: normalize the actions after translation > > When all OF actions have been translated, there could be actions at > the end of list of odp actions which are not needed to be executed. > So, the list can be normalized at the end of xlate_actions(). > > Signed-off-by: Zoltan Balogh <[email protected]> > Signed-off-by: Sugesh Chandran <[email protected]> > Co-authored-by: Sugesh Chandran <[email protected]> > CC: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 67 > ++++++++++++++++++++++++++++++++++++++++++++ > tests/ofproto-dpif.at | 48 +++++++++++++++++++++++++++++++ > 2 files changed, 115 insertions(+) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 36d0a0e1f..2af1ec1e8 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -6954,6 +6954,70 @@ xlate_wc_finish(struct xlate_ctx *ctx) > } > } > > +/* Returns true if the action stored in 'nla' can be a valid last action of a > + * datapath flow. */ > +static bool > +is_valid_last_action(const struct nlattr *nla) > +{ > + enum ovs_action_attr action_type = nl_attr_type(nla); > + > + switch (action_type) { > + case OVS_ACTION_ATTR_USERSPACE: > + case OVS_ACTION_ATTR_SAMPLE: > + case OVS_ACTION_ATTR_TRUNC: > + case OVS_ACTION_ATTR_RECIRC: > + case OVS_ACTION_ATTR_TUNNEL_PUSH: > + case OVS_ACTION_ATTR_TUNNEL_POP: > + case OVS_ACTION_ATTR_OUTPUT: > + case OVS_ACTION_ATTR_CLONE: > + case OVS_ACTION_ATTR_CT: > + return true; > + case OVS_ACTION_ATTR_UNSPEC: > + case OVS_ACTION_ATTR_SET: > + case OVS_ACTION_ATTR_PUSH_VLAN: > + case OVS_ACTION_ATTR_POP_VLAN: > + case OVS_ACTION_ATTR_HASH: > + case OVS_ACTION_ATTR_PUSH_MPLS: > + case OVS_ACTION_ATTR_POP_MPLS: > + case OVS_ACTION_ATTR_SET_MASKED: > + case OVS_ACTION_ATTR_PUSH_ETH: > + case OVS_ACTION_ATTR_POP_ETH: > + case OVS_ACTION_ATTR_METER: > + case OVS_ACTION_ATTR_ENCAP_NSH: > + case OVS_ACTION_ATTR_DECAP_NSH: > + case __OVS_ACTION_ATTR_MAX: > + default: > + return false; > + } > +} > + > +/* Returns offset of last netlink attribute storing valid action in array > + * 'data'. Execution of actions beyond this last attribute does not make > sense. > + */ > +static size_t > +last_action_offset(const struct nlattr *data, const size_t data_len) > +{ > + const struct nlattr *last = data; > + > + uint16_t left; > + const struct nlattr *a; > + NL_ATTR_FOR_EACH (a, left, data, data_len) { > + if (is_valid_last_action(a)) { > + last = nl_attr_next(a); > + } > + } > + > + return (char *) last - (char *) data; > +} > + > +/* Get rid of any unneeded actions at the tail end. */ > +static void > +normalize_odp_actions(struct xlate_ctx *ctx) > +{ > + struct ofpbuf *oa = ctx->odp_actions; > + oa->size = last_action_offset(oa->data, oa->size); > +} > + > /* Translates the flow, actions, or rule in 'xin' into datapath actions in > * 'xout'. > * The caller must take responsibility for eventually freeing 'xout', with > @@ -7364,7 +7428,10 @@ exit: > if (xin->odp_actions) { > ofpbuf_clear(xin->odp_actions); > } > + } else { > + normalize_odp_actions(&ctx); > } > + > return ctx.error; > } > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index e7df1504a..5bcd8bb46 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -10133,3 +10133,51 @@ AT_CHECK([grep "Datapath actions" stdout], [0], > > OVS_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - xlate error - normalize actions]) > + > +# ->-+ > +# | p1 > +# +-o-------+ > +# | br0 | > +# +-o-----o-+ > +# patch0| |patch1 > +# +-->--+ > + > +OVS_VSWITCHD_START([dnl > + -- add-port br0 patch0 \ > + -- set interface patch0 type=patch options:peer=patch1 ofport_request=10 > \ > + -- add-port br0 patch1 \ > + -- set interface patch1 type=patch options:peer=patch0 ofport_request=20 > +]) > + > +AT_CHECK([ > + ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy ofport_request=1 > +]) > + > +AT_CHECK([ > + ovs-appctl dpif/show | grep -v hit | sed "s/$(printf \\t)/ /g" | sed > 's./[[0-9]]\{1,\}..' > +], [0], [dnl > + br0: > + br0 65534: (dummy-internal) > + p1 1: (dummy) > + patch0 10/none: (patch: peer=patch1) > + patch1 20/none: (patch: peer=patch0) > +]) > + > +# Error due to pushing too many MPLS labels. > +AT_CHECK([ > + ovs-ofctl del-flows br0 > + ovs-ofctl add-flow br0 > "table=0,in_port=1,actions=mod_dl_src:aa:aa:aa:bb:bb:00,goto_table:1" > -OOpenFlow13 > + ovs-ofctl add-flow br0 "table=0,in_port=patch1,actions=goto_table:2" > -OOpenFlow13 > + ovs-ofctl add-flow br0 > "table=1,actions=push_vlan:0x8100,set_field:4196->vlan_vid,output:patch0" > -OOpenFlow13 > + ovs-ofctl add-flow br0 "table=2,actions=push_mpls:0x8847,output:patch0" > +], [0]) > + > +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,ip'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: drop > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > -- > 2.14.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
