From 5af43325c9f78cd4617ee3dc5c7aa746e9c2a2c0 Mon Sep 17 00:00:00 2001 From: Pravin B Shelar Date: Tue, 25 Nov 2014 07:39:20 -0800 Subject: [PATCH] ofproto-dpif: Fix MPLS multiple Push pop action. vSwitchd does not generate correct MPLS actions for multiple MPLS push or pop action. Datapath can handle multiple push action for in single action list. But for after first MPLS pop it needs to recirculate packet to refill packet key. Following patch fixes it accordingly. Reported-by: Stefano Salsano Signed-off-by: Pravin B Shelar Tested-by: Pier Luigi Ventre Acked-by: Jarno Rajahalme --- lib/flow.c | 11 +++++--- ofproto/ofproto-dpif-xlate.c | 24 ++++++++--------- tests/automake.mk | 1 + tests/mpls-xlate.at | 50 ++++++++++++++++++++++++++++++++++++ tests/odp.at | 2 -- tests/ofproto-dpif.at | 42 +++++++++++++++--------------- tests/testsuite.at | 1 + 7 files changed, 92 insertions(+), 39 deletions(-) create mode 100644 tests/mpls-xlate.at diff --git a/lib/flow.c b/lib/flow.c index 521ee828a..eb7fdf145 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -1422,18 +1422,21 @@ flow_count_mpls_labels(const struct flow *flow, struct flow_wildcards *wc) /* dl_type is always masked. */ if (eth_type_mpls(flow->dl_type)) { int i; - int len = FLOW_MAX_MPLS_LABELS; + int cnt; - for (i = 0; i < len; i++) { + cnt = 0; + for (i = 0; i < FLOW_MAX_MPLS_LABELS; i++) { if (wc) { wc->masks.mpls_lse[i] |= htonl(MPLS_BOS_MASK); } if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) { return i + 1; } + if (flow->mpls_lse[i]) { + cnt++; + } } - - return len; + return cnt; } else { return 0; } diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index c1327a6b3..eb8731553 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3274,9 +3274,6 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls) } ctx->exit = true; return; - } else if (n >= ctx->xbridge->max_mpls_depth) { - COVERAGE_INC(xlate_actions_mpls_overflow); - ctx->xout->slow |= SLOW_ACTION; } flow_push_mpls(flow, n, mpls->ethertype, wc); @@ -3290,7 +3287,7 @@ compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type) int n = flow_count_mpls_labels(flow, wc); if (flow_pop_mpls(flow, n, eth_type, wc)) { - if (ctx->xbridge->enable_recirc && !eth_type_mpls(eth_type)) { + if (ctx->xbridge->enable_recirc) { ctx->was_mpls = true; } } else if (n >= FLOW_MAX_MPLS_LABELS) { @@ -3698,12 +3695,15 @@ xlate_action_set(struct xlate_ctx *ctx) } static bool -ofpact_needs_recirculation_after_mpls(const struct xlate_ctx *ctx, - const struct ofpact *a) +ofpact_needs_recirculation_after_mpls(const struct ofpact *a, struct xlate_ctx *ctx) { struct flow_wildcards *wc = &ctx->xout->wc; struct flow *flow = &ctx->xin->flow; + if (!ctx->was_mpls) { + return false; + } + switch (a->type) { case OFPACT_OUTPUT: case OFPACT_GROUP: @@ -3718,11 +3718,6 @@ ofpact_needs_recirculation_after_mpls(const struct xlate_ctx *ctx, case OFPACT_SET_TUNNEL: case OFPACT_SET_QUEUE: case OFPACT_POP_QUEUE: - case OFPACT_POP_MPLS: - case OFPACT_DEC_MPLS_TTL: - case OFPACT_SET_MPLS_TTL: - case OFPACT_SET_MPLS_TC: - case OFPACT_SET_MPLS_LABEL: case OFPACT_NOTE: case OFPACT_OUTPUT_REG: case OFPACT_EXIT: @@ -3733,6 +3728,11 @@ ofpact_needs_recirculation_after_mpls(const struct xlate_ctx *ctx, case OFPACT_SAMPLE: return false; + case OFPACT_POP_MPLS: + case OFPACT_DEC_MPLS_TTL: + case OFPACT_SET_MPLS_TTL: + case OFPACT_SET_MPLS_TC: + case OFPACT_SET_MPLS_LABEL: case OFPACT_SET_IPV4_SRC: case OFPACT_SET_IPV4_DST: case OFPACT_SET_IP_DSCP: @@ -3794,7 +3794,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; } - if (ctx->was_mpls && ofpact_needs_recirculation_after_mpls(ctx, a)) { + if (ofpact_needs_recirculation_after_mpls(a, ctx)) { compose_recirculate_action(ctx, ofpacts, a, ofpacts_len); return; } diff --git a/tests/automake.mk b/tests/automake.mk index 7422f970f..33502bc1b 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -20,6 +20,7 @@ TESTSUITE_AT = \ tests/ofp-errors.at \ tests/ovs-ofctl.at \ tests/odp.at \ + tests/mpls-xlate.at \ tests/multipath.at \ tests/bfd.at \ tests/cfm.at \ diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at new file mode 100644 index 000000000..e2ef2e7c6 --- /dev/null +++ b/tests/mpls-xlate.at @@ -0,0 +1,50 @@ +AT_BANNER([mpls_xlate]) + +AT_SETUP([MPLS xlate action]) + +OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1]) + +AT_CHECK([ovs-appctl dpif/show], [0], [dnl +dummy@ovs-dummy: hit:0 missed:0 + br0: + br0 65534/100: (dummy) + p0 1/1: (dummy) +]) + +dnl Setup single MPLS tags. +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=local,dl_type=0x0800,action=push_mpls:0x8847,set_field:10-\>mpls_label,output:1]) +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 dl_type=0x8847,in_port=1,mpls_label=20,action=pop_mpls:0x0800,output:LOCAL]) + +dnl Test MPLS push +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=1.1.2.92,dst=1.1.2.88,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: push_mpls(label=10,tc=0,ttl=64,bos=1,eth_type=0x8847),1 +]) + +dnl Test MPLS pop +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x8847),mpls(label=20,tc=0,ttl=64,bos=1)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: pop_mpls(eth_type=0x800),100 +]) + +dnl Setup multiple MPLS tags. +AT_CHECK([ovs-ofctl del-flows br0]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=local,dl_type=0x0800,action=push_mpls:0x8847,set_field:10-\>mpls_label,push_mpls:0x8847,set_field:20-\>mpls_label,output:1]) +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 table=0,dl_type=0x8847,in_port=1,mpls_label=60,action=pop_mpls:0x8847,goto_table:1]) +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 table=1,dl_type=0x8847,in_port=1,mpls_label=50,action=pop_mpls:0x0800,output:LOCAL]) + +dnl Double MPLS push +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=1.1.2.92,dst=1.1.2.88,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: push_mpls(label=10,tc=0,ttl=64,bos=1,eth_type=0x8847),push_mpls(label=20,tc=0,ttl=64,bos=0,eth_type=0x8847),1 +]) + +dnl Double MPLS pop +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x8847),mpls(label=60,tc=0/0,ttl=64,bos=0)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: pop_mpls(eth_type=0x8847),recirc(300) +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/tests/odp.at b/tests/odp.at index f76aaa140..8f96c6a7d 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -84,8 +84,6 @@ dnl Internally a stack of 3 LSEs will be used with the trailing LSEs dnl set to zero. This is reflected when the key is formated sed '/bos=0/{ s/^/ODP_FIT_TOO_LITTLE: / -s/mpls(label=100,tc=7,ttl=100,bos=0)/mpls(lse0=0x64e64,lse1=0,lse2=0)/ -s/mpls(label=1000,tc=4,ttl=200,bos=0)/mpls(lse0=0x3e88c8,lse1=0,lse2=0)/ }' < odp-in.txt > odp-out.txt AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index baa942fcf..231c57fb7 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -2054,13 +2054,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:01:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:01:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:01:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 ]) @@ -2108,13 +2108,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=58 in_port=1 (via action) data_len=58 (unbuffered) tcp,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:02:00,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=255,tp_src=80,tp_dst=0,tcp_flags=0 tcp_csum:7744 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=58 in_port=1 (via action) data_len=58 (unbuffered) tcp,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:02:00,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=255,tp_src=80,tp_dst=0,tcp_flags=0 tcp_csum:7744 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=58 in_port=1 (via action) data_len=58 (unbuffered) tcp,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:02:00,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=255,tp_src=80,tp_dst=0,tcp_flags=0 tcp_csum:7744 ]) @@ -2190,13 +2190,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:00,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:00,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:00,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=30,mpls_bos=1 ]) @@ -2218,13 +2218,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 ]) @@ -2246,13 +2246,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:03:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=29,mpls_bos=1 ]) @@ -2376,13 +2376,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mplsm,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:01,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 ]) @@ -2402,13 +2402,13 @@ OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6]) OVS_APP_EXIT_AND_WAIT(ovs-ofctl) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=62 in_port=1 (via action) data_len=62 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=254 cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) mpls,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:05:10,dl_dst=50:54:00:00:00:07,mpls_label=20,mpls_tc=0,mpls_ttl=31,mpls_bos=1 ]) diff --git a/tests/testsuite.at b/tests/testsuite.at index d13732f64..3792328f4 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -132,6 +132,7 @@ m4_include([tests/ofp-util.at]) m4_include([tests/ofp-errors.at]) m4_include([tests/ovs-ofctl.at]) m4_include([tests/odp.at]) +m4_include([tests/mpls-xlate.at]) m4_include([tests/multipath.at]) m4_include([tests/learn.at]) m4_include([tests/vconn.at]) -- 2.20.1