Currently in OvS if we hit "Table-miss" rules (associated with Controller action) then we send PACKET_IN message to controller with reason as OFPR_NO_MATCH.
“Table-miss” rule is one whose priority is 0 and its catch all rule. But if we hit same "Table-miss" rule after executing group entry we will send the reason as OFPR_ACTION (for OF1.3 and below) and OFPR_GROUP (for OF1.4 and above). This is because once we execute group entry we set ctx->in_group and later when we hit the "Table-miss" rule, Since ctx->in_group is set we send reason as OFPR_ACTION (for OF1.3) and OFPR_GROUP (for OF1.4 and above). For eg: for the following pipeline, we will send the reason as OFPR_ACTION even if we hit The “Table-miss” rule. cookie=0x8000000, duration=761.189s, table=0, n_packets=1401, n_bytes=67954, priority=4,in_port=9,vlan_tci=0x0000/0x1fff actions=write_metadata:0x67870000000000/0xffffff0000000001,goto_table:17 cookie=0x6800001, duration=768.848s, table=17, n_packets=1418, n_bytes=68776, priority=10,metadata=0x67870000000000/0xffffff0000000000 actions=write_metadata:0xe067870000000000/0xfffffffffffffffe,goto_table:60 cookie=0x6800000, duration=24944.312s, table=60, n_packets=58244, n_bytes=2519520, priority=0 actions=resubmit(,17) cookie=0x8040000, duration=785.733s, table=17, n_packets=1450, n_bytes=69724, priority=10,metadata=0xe067870000000000/0xffffff0000000000 actions=write_metadata:0x67871d4d000000/0xfffffffffffffffe,goto_table:43 cookie=0x822002d, duration=24960.795s, table=43, n_packets=53097, n_bytes=2230074, priority=100,arp,arp_op=1 actions=group:6000 group_id=6000,type=all,bucket=actions=CONTROLLER:65535, bucket=actions=resubmit(,48), bucket=actions=resubmit(,81) cookie=0x8500000, duration=24977.323s, table=48, n_packets=58309, n_bytes=2522634, priority=0 actions=resubmit(,49),resubmit(,50) cookie=0x8050000, duration=24984.679s, table=50, n_packets=6, n_bytes=264, priority=0 actions=CONTROLLER:65535 Currently we are sending table_id as 50 and packet_in reason as OFPR_ACTION. Instead of sending packet_in reason as OFPR_NO_MATCH. Signed-off-by: Keshav Gupta <[email protected]> Co-authored-by: Rohith Basavaraja <[email protected]> Signed-off-by: Rohith Basavaraja <[email protected]> --- ofproto/ofproto-dpif-xlate.c | 82 +++++++++++++++++++++++++------------------- tests/ofproto-dpif.at | 38 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 36 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index c02a032..0215793 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -249,7 +249,6 @@ struct xlate_ctx { */ int depth; /* Current resubmit nesting depth. */ int resubmits; /* Total number of resubmits. */ - bool in_group; /* Currently translating ofgroup, if true. */ bool in_action_set; /* Currently translating action_set, if true. */ bool in_packet_out; /* Currently translating a packet_out msg, if * true. */ @@ -528,12 +527,12 @@ static OVSRCU_TYPE(struct xlate_cfg *) xcfgp = OVSRCU_INITIALIZER(NULL); static struct xlate_cfg *new_xcfg = NULL; typedef void xlate_actions_handler(const struct ofpact *, size_t ofpacts_len, - struct xlate_ctx *, bool); + struct xlate_ctx *, bool, bool); static bool may_receive(const struct xport *, struct xlate_ctx *); static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len, - struct xlate_ctx *, bool); + struct xlate_ctx *, bool, bool); static void clone_xlate_actions(const struct ofpact *, size_t ofpacts_len, - struct xlate_ctx *, bool); + struct xlate_ctx *, bool, bool); static void xlate_normal(struct xlate_ctx *); static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port, uint8_t table_id, bool may_packet_in, @@ -4104,7 +4103,7 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule, ctx->rule_cookie = rule->up.flow_cookie; actions = rule_get_actions(&rule->up); actions_xlator(actions->ofpacts, actions->ofpacts_len, ctx, - is_last_action); + is_last_action, false); ctx->rule_cookie = old_cookie; ctx->rule = old_rule; ctx->depth -= deepens; @@ -4279,7 +4278,8 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket, ofpacts_execute_action_set(&action_list, &action_set); ctx->depth++; - do_xlate_actions(action_list.data, action_list.size, ctx, is_last_action); + do_xlate_actions(action_list.data, action_list.size, ctx, is_last_action, + true); ctx->depth--; ofpbuf_uninit(&action_list); @@ -4459,9 +4459,6 @@ static void xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group, bool is_last_action) { - bool was_in_group = ctx->in_group; - ctx->in_group = true; - if (group->up.type == OFPGT11_ALL || group->up.type == OFPGT11_INDIRECT) { struct ovs_list *last_bucket = ovs_list_back(&group->up.buckets); struct ofputil_bucket *bucket; @@ -4492,8 +4489,6 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group, } } } - - ctx->in_group = was_in_group; } static bool @@ -4958,7 +4953,8 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx) static void xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port, uint16_t controller_len, bool may_packet_in, - bool is_last_action, bool truncate) + bool is_last_action, bool truncate, + bool group_bucket_action) { ofp_port_t prev_nf_output_iface = ctx->nf_output_iface; @@ -4986,7 +4982,7 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port, case OFPP_CONTROLLER: xlate_controller_action(ctx, controller_len, (ctx->in_packet_out ? OFPR_PACKET_OUT - : ctx->in_group ? OFPR_GROUP + : group_bucket_action ? OFPR_GROUP : ctx->in_action_set ? OFPR_ACTION_SET : OFPR_ACTION), 0, NULL, 0); @@ -5016,7 +5012,8 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port, static void xlate_output_reg_action(struct xlate_ctx *ctx, const struct ofpact_output_reg *or, - bool is_last_action) + bool is_last_action, + bool group_bucket_action) { uint64_t port = mf_get_subfield(&or->src, &ctx->xin->flow); if (port <= UINT16_MAX) { @@ -5027,7 +5024,8 @@ xlate_output_reg_action(struct xlate_ctx *ctx, memset(&value, 0xff, sizeof value); mf_write_subfield_flow(&or->src, &value, &ctx->wc->masks); xlate_output_action(ctx, u16_to_ofp(port), or->max_len, - false, is_last_action, false); + false, is_last_action, false, + group_bucket_action); } else { xlate_report(ctx, OFT_WARN, "output port %"PRIu64" is out of range", port); @@ -5037,7 +5035,8 @@ xlate_output_reg_action(struct xlate_ctx *ctx, static void xlate_output_trunc_action(struct xlate_ctx *ctx, ofp_port_t port, uint32_t max_len, - bool is_last_action) + bool is_last_action, + bool group_bucket_action) { bool support_trunc = ctx->xbridge->support.trunc; struct ovs_action_trunc *trunc; @@ -5074,7 +5073,8 @@ xlate_output_trunc_action(struct xlate_ctx *ctx, OVS_ACTION_ATTR_TRUNC, sizeof *trunc); trunc->max_len = max_len; - xlate_output_action(ctx, port, 0, false, is_last_action, true); + xlate_output_action(ctx, port, 0, false, is_last_action, true, + group_bucket_action); if (!support_trunc) { ctx->xout->slow |= SLOW_ACTION; } @@ -5088,7 +5088,8 @@ xlate_output_trunc_action(struct xlate_ctx *ctx, static void xlate_enqueue_action(struct xlate_ctx *ctx, const struct ofpact_enqueue *enqueue, - bool is_last_action) + bool is_last_action, + bool group_bucket_action) { ofp_port_t ofp_port = enqueue->port; uint32_t queue_id = enqueue->queue; @@ -5100,7 +5101,8 @@ xlate_enqueue_action(struct xlate_ctx *ctx, if (error) { /* Fall back to ordinary output action. */ xlate_output_action(ctx, enqueue->port, 0, false, - is_last_action, false); + is_last_action, false, + group_bucket_action); return; } @@ -5163,7 +5165,8 @@ slave_enabled_cb(ofp_port_t ofp_port, void *xbridge_) static void xlate_bundle_action(struct xlate_ctx *ctx, const struct ofpact_bundle *bundle, - bool is_last_action) + bool is_last_action, + bool group_bucket_action) { ofp_port_t port; @@ -5173,7 +5176,8 @@ xlate_bundle_action(struct xlate_ctx *ctx, nxm_reg_load(&bundle->dst, ofp_to_u16(port), &ctx->xin->flow, ctx->wc); xlate_report_subfield(ctx, &bundle->dst); } else { - xlate_output_action(ctx, port, 0, false, is_last_action, false); + xlate_output_action(ctx, port, 0, false, is_last_action, false, + group_bucket_action); } } @@ -5472,7 +5476,8 @@ reversible_actions(const struct ofpact *ofpacts, size_t ofpacts_len) static void clone_xlate_actions(const struct ofpact *actions, size_t actions_len, - struct xlate_ctx *ctx, bool is_last_action) + struct xlate_ctx *ctx, bool is_last_action, + bool group_bucket_action OVS_UNUSED) { struct ofpbuf old_stack = ctx->stack; union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)]; @@ -5489,7 +5494,7 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len, if (reversible_actions(actions, actions_len) || is_last_action) { old_flow = ctx->xin->flow; - do_xlate_actions(actions, actions_len, ctx, is_last_action); + do_xlate_actions(actions, actions_len, ctx, is_last_action, false); if (!ctx->freezing) { xlate_action_set(ctx); } @@ -5513,7 +5518,7 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len, if (ctx->xbridge->support.clone) { /* Use clone action */ /* Use clone action as datapath clone. */ offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE); - do_xlate_actions(actions, actions_len, ctx, true); + do_xlate_actions(actions, actions_len, ctx, true, false); if (!ctx->freezing) { xlate_action_set(ctx); } @@ -5529,7 +5534,7 @@ clone_xlate_actions(const struct ofpact *actions, size_t actions_len, offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE); ac_offset = nl_msg_start_nested(ctx->odp_actions, OVS_SAMPLE_ATTR_ACTIONS); - do_xlate_actions(actions, actions_len, ctx, true); + do_xlate_actions(actions, actions_len, ctx, true, false); if (!ctx->freezing) { xlate_action_set(ctx); } @@ -5576,7 +5581,8 @@ compose_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc, { size_t oc_actions_len = ofpact_nest_get_action_len(oc); - clone_xlate_actions(oc->actions, oc_actions_len, ctx, is_last_action); + clone_xlate_actions(oc->actions, oc_actions_len, ctx, is_last_action, + false); } static void @@ -5658,7 +5664,7 @@ xlate_action_set(struct xlate_ctx *ctx) struct ovs_list *old_trace = ctx->xin->trace; ctx->xin->trace = xlate_report(ctx, OFT_TABLE, "--. Executing action set:"); - do_xlate_actions(action_list.data, action_list.size, ctx, true); + do_xlate_actions(action_list.data, action_list.size, ctx, true, false); ctx->xin->trace = old_trace; ctx->in_action_set = false; @@ -5901,7 +5907,7 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc, ctx->wc->masks.ct_mark = 0; ctx->wc->masks.ct_label = OVS_U128_ZERO; do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx, - is_last_action); + is_last_action, false); if (ofc->zone_src.field) { zone = mf_get_subfield(&ofc->zone_src, &ctx->xin->flow); @@ -6313,7 +6319,8 @@ xlate_ofpact_unroll_xlate(struct xlate_ctx *ctx, static void do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, - struct xlate_ctx *ctx, bool is_last_action) + struct xlate_ctx *ctx, bool is_last_action, + bool group_bucket_action) { struct flow_wildcards *wc = ctx->wc; struct flow *flow = &ctx->xin->flow; @@ -6362,7 +6369,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_OUTPUT: xlate_output_action(ctx, ofpact_get_OUTPUT(a)->port, ofpact_get_OUTPUT(a)->max_len, true, last, - false); + false, group_bucket_action); break; case OFPACT_GROUP: @@ -6393,7 +6400,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_ENQUEUE: memset(&wc->masks.skb_priority, 0xff, sizeof wc->masks.skb_priority); - xlate_enqueue_action(ctx, ofpact_get_ENQUEUE(a), last); + xlate_enqueue_action(ctx, ofpact_get_ENQUEUE(a), last, + group_bucket_action); break; case OFPACT_SET_VLAN_VID: @@ -6609,16 +6617,19 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_BUNDLE: - xlate_bundle_action(ctx, ofpact_get_BUNDLE(a), last); + xlate_bundle_action(ctx, ofpact_get_BUNDLE(a), last, + group_bucket_action); break; case OFPACT_OUTPUT_REG: - xlate_output_reg_action(ctx, ofpact_get_OUTPUT_REG(a), last); + xlate_output_reg_action(ctx, ofpact_get_OUTPUT_REG(a), last, + group_bucket_action); break; case OFPACT_OUTPUT_TRUNC: xlate_output_trunc_action(ctx, ofpact_get_OUTPUT_TRUNC(a)->port, - ofpact_get_OUTPUT_TRUNC(a)->max_len, last); + ofpact_get_OUTPUT_TRUNC(a)->max_len, last, + group_bucket_action); break; case OFPACT_LEARN: @@ -7036,7 +7047,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) .depth = xin->depth, .resubmits = xin->resubmits, - .in_group = false, .in_action_set = false, .in_packet_out = xin->in_packet_out, .pending_encap = false, @@ -7285,7 +7295,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) } mirror_ingress_packet(&ctx); - do_xlate_actions(ofpacts, ofpacts_len, &ctx, true); + do_xlate_actions(ofpacts, ofpacts_len, &ctx, true, false); if (ctx.error) { goto exit; } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index d798925..caa9b43 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -3421,6 +3421,44 @@ OFPST_FLOW reply (OF1.4): OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - packet-in reasons (Openflow 1.3)]) + +OVS_VSWITCHD_START([dnl + set bridge br0 datapath_type=dummy \ + protocols=OpenFlow10,OpenFlow13,OpenFlow14,OpenFlow15 -- \ + add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1]) + +AT_CHECK([ + ovs-ofctl -OOpenFlow13 del-flows br0 + ovs-ofctl -OOpenFlow13 add-group br0 "group_id=6000,type=all,bucket=actions=controller,bucket=actions=resubmit(,48),bucket=actions=resubmit(,81)" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=0, in_port=1, vlan_tci=0x0000/0x1fff actions=write_metadata:0x67870000000000/0xffffff0000000001,goto_table:17" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=17, priority=10,metadata=0x67870000000000/0xffffff0000000000 actions=write_metadata:0xe067870000000000/0xfffffffffffffffe,goto_table:60" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=60, priority=0 actions=resubmit(,17)" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=17, priority=10,metadata=0xe067870000000000/0xffffff0000000000 actions=write_metadata:0x67871d4d000000/0xfffffffffffffffe,goto_table:43" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=43, priority=100,icmp actions=group:6000" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=48, priority=0 actions=resubmit(,49),resubmit(,50)" + ovs-ofctl -OOpenFlow13 add-flow br0 "table=50, priority=0 actions=controller" +], [0], [ignore]) + +AT_CHECK([ovs-ofctl monitor -OOpenFlow13 -P standard br0 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +AT_CHECK([ + ovs-appctl netdev-dummy/receive p1 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 +], [0], [ignore]) + +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 1]) +OVS_APP_EXIT_AND_WAIT(ovs-ofctl) + +AT_CHECK([cat ofctl_monitor.log], [0], [dnl +OFPT_PACKET_IN (OF1.3) (xid=0x0): table_id=43 cookie=0x0 total_len=98 metadata=0x67871d4d000000,in_port=1 (via action) data_len=98 (unbuffered) +icmp,vlan_tci=0x0000,dl_src=3a:6d:d2:09:9c:ab,dl_dst=1e:2c:e9:2a:66:9e,nw_src=192.168.10.10,nw_dst=192.168.10.30,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0 icmp_csum:6f20 +dnl +OFPT_PACKET_IN (OF1.3) (xid=0x0): table_id=50 cookie=0x0 total_len=98 metadata=0x67871d4d000000,in_port=1 (via no_match) data_len=98 (unbuffered) +icmp,vlan_tci=0x0000,dl_src=3a:6d:d2:09:9c:ab,dl_dst=1e:2c:e9:2a:66:9e,nw_src=192.168.10.10,nw_dst=192.168.10.30,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0 icmp_csum:6f20 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP AT_SETUP([ofproto-dpif - ARP modification slow-path]) OVS_VSWITCHD_START -- 1.9.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
