[ovs-dev] [PATCH v1] ofproto-dpif-xlate: Fix packet_in reason for Table-miss rule
Keshav Gupta
keshav.gupta at ericsson.com
Tue Jul 3 17:44:45 UTC 2018
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 <keshav.gupta at ericsson.com>
Co-authored-by: Rohith Basavaraja <rohith.basavaraja at gmail.com>
Signed-off-by: Rohith Basavaraja <rohith.basavaraja at gmail.com>
---
ofproto/ofproto-dpif-xlate.c | 75 +++++++++++++++++++++++++++-----------------
tests/ofproto-dpif.at | 38 ++++++++++++++++++++++
2 files changed, 84 insertions(+), 29 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c02a032..793b820 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -528,12 +528,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 +4104,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 +4279,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);
@@ -4958,7 +4959,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 +4988,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 +5018,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 +5030,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 +5041,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 +5079,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 +5094,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 +5107,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 +5171,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 +5182,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 +5482,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 +5500,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 +5524,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 +5540,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 +5587,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 +5670,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 +5913,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 +6325,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 +6375,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 +6406,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 +6623,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:
@@ -7285,7 +7302,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
More information about the dev
mailing list