From 999f0d4588503af5a7645ed2f64764bd48c29b9b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 13 Apr 2010 10:12:25 -0700 Subject: [PATCH] ofproto: Make NXAST_RESUBMIT take header modifications into account. Until now, the NXAST_RESUBMIT action has always looked up the original flow except for the updated in_port. This commit changes the semantics to instead look up the flow as modified by any preceding actions that affect it, e.g. if OFPAT_SET_VLAN_VID precedes NXAST_RESUBMIT, then NXAST_RESUBMIT now looks up the flow with the modified VLAN, not the original (as well as the modified in_port). Also, document how NXAST_RESUBMIT is supposed to work. Suggested-by: Paul Ingram --- include/openflow/nicira-ext.h | 23 ++++++++++++++++++++++- ofproto/ofproto.c | 23 ++++++++++++++++------- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 7232d570..535cfc3a 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -57,7 +57,28 @@ OFP_ASSERT(sizeof(struct nicira_header) == 16); enum nx_action_subtype { NXAST_SNAT__OBSOLETE, /* No longer used. */ - NXAST_RESUBMIT /* Throw against flow table again. */ + + /* Searches the flow table again, using a flow that is slightly modified + * from the original lookup: + * + * - The flow's in_port is changed to that specified in the 'in_port' + * member of struct nx_action_resubmit. + * + * - If NXAST_RESUBMIT is preceded by actions that affect the flow + * (e.g. OFPAT_SET_VLAN_VID), then the flow is updated with the new + * values. + * + * If the modified flow matches in the flow table, then the corresponding + * actions are executed, except that NXAST_RESUBMIT actions found in the + * secondary set of actions are ignored. Afterward, actions following + * NXAST_RESUBMIT in the original set of actions, if any, are executed; any + * changes made to the packet (e.g. changes to VLAN) by secondary actions + * persist when those actions are executed, although the original in_port + * is restored. + * + * NXAST_RESUBMIT may be used any number of times within a set of actions. + */ + NXAST_RESUBMIT }; /* Action structure for NXAST_RESUBMIT. */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9ddf6b15..0f4ba684 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2150,6 +2150,9 @@ xlate_nicira_action(struct action_xlate_ctx *ctx, xlate_table_action(ctx, ofp_port_to_odp_port(ntohs(nar->in_port))); break; + /* If you add a new action here that modifies flow data, don't forget to + * update the flow key in ctx->flow in the same key. */ + default: VLOG_DBG_RL(&rl, "unknown Nicira action type %"PRIu16, subtype); break; @@ -2183,53 +2186,59 @@ do_xlate_actions(const union ofp_action *in, size_t n_in, case OFPAT_SET_VLAN_VID: oa = odp_actions_add(ctx->out, ODPAT_SET_VLAN_VID); - oa->vlan_vid.vlan_vid = ia->vlan_vid.vlan_vid; + ctx->flow.dl_vlan = oa->vlan_vid.vlan_vid = ia->vlan_vid.vlan_vid; break; case OFPAT_SET_VLAN_PCP: oa = odp_actions_add(ctx->out, ODPAT_SET_VLAN_PCP); - oa->vlan_pcp.vlan_pcp = ia->vlan_pcp.vlan_pcp; + ctx->flow.dl_vlan_pcp = oa->vlan_pcp.vlan_pcp = ia->vlan_pcp.vlan_pcp; break; case OFPAT_STRIP_VLAN: odp_actions_add(ctx->out, ODPAT_STRIP_VLAN); + ctx->flow.dl_vlan = OFP_VLAN_NONE; + ctx->flow.dl_vlan_pcp = 0; break; case OFPAT_SET_DL_SRC: oa = odp_actions_add(ctx->out, ODPAT_SET_DL_SRC); memcpy(oa->dl_addr.dl_addr, ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); + memcpy(ctx->flow.dl_src, + ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); break; case OFPAT_SET_DL_DST: oa = odp_actions_add(ctx->out, ODPAT_SET_DL_DST); memcpy(oa->dl_addr.dl_addr, ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); + memcpy(ctx->flow.dl_dst, + ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); break; case OFPAT_SET_NW_SRC: oa = odp_actions_add(ctx->out, ODPAT_SET_NW_SRC); - oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; + ctx->flow.nw_src = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; break; case OFPAT_SET_NW_DST: oa = odp_actions_add(ctx->out, ODPAT_SET_NW_DST); - oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; + ctx->flow.nw_dst = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; break; case OFPAT_SET_NW_TOS: oa = odp_actions_add(ctx->out, ODPAT_SET_NW_TOS); - oa->nw_tos.nw_tos = ia->nw_tos.nw_tos; + ctx->flow.nw_tos = oa->nw_tos.nw_tos = ia->nw_tos.nw_tos; break; case OFPAT_SET_TP_SRC: oa = odp_actions_add(ctx->out, ODPAT_SET_TP_SRC); - oa->tp_port.tp_port = ia->tp_port.tp_port; + ctx->flow.tp_src = oa->tp_port.tp_port = ia->tp_port.tp_port; break; case OFPAT_SET_TP_DST: oa = odp_actions_add(ctx->out, ODPAT_SET_TP_DST); - oa->tp_port.tp_port = ia->tp_port.tp_port; + ctx->flow.tp_dst = oa->tp_port.tp_port = ia->tp_port.tp_port; break; case OFPAT_VENDOR: -- 2.30.2