[ Cc Pravin B Shelar ]
On Tue, Mar 05, 2013 at 06:11:27PM -0800, Ben Pfaff wrote:
> On Wed, Mar 06, 2013 at 10:42:02AM +0900, Simon Horman wrote:
> > On Tue, Mar 05, 2013 at 10:14:44AM -0800, Ben Pfaff wrote:
> > > On Thu, Feb 28, 2013 at 06:15:07PM +0900, Simon Horman wrote:
> > > > This adds support for the OpenFlow 1.1+ dec_mpls_ttl action.
> > > > And also adds an NX dec_mpls_ttl action.
> > > >
> > > > The handling of the TTL modification is entirely handled in userspace.
> > > >
> > > > Reviewed-by: Isaku Yamahata <yamah...@valinux.co.jp>
> > > > Signed-off-by: Simon Horman <ho...@verge.net.au>
> > >
> > > The only issue I see with this is that it seems uncertain about what is
> > > an invalid MPLS TTL. The code says that, pre-decrement, values 0 and 1
> > > are invalid:
> > >
> > > + if (ttl > 1) {
> > > + ttl--;
> > > + set_mpls_lse_ttl(&ctx->flow.mpls_lse, ttl);
> > > + return false;
> > > + } else {
> > > + execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
> > >
> > > The documentation says that, pre-decrement, value 0 is invalid:
> > >
> > > +.IP \fBdec_mpls_ttl\fR
> > > +Decrement TTL of the outer MPLS label stack entry of a packet. If the
> > > TTL
> > > +is initially zero, no decrement occurs. Instead, a ``packet-in'' message
> > >
> > > I don't know MPLS, so I don't know which definition is correct.
> >
> > I am not sure and to be honest I had just followed dec_ttl implementation
> > when adding the code. Reading up I think that section 2.3 of RFC3443
> > implies this is the desired behaviour.
> >
> > I'm not sure why the documentation conflicts with the code, most likely
> > it documents a previous incantation of the code. In any case I propose
> > updating it rather than the code.
>
> OK, that makes sense, thank you.
It seems to me that dec_ttl has the same problem.
But it also seems to me that it is the code rather than the documentation
that should be updated.
The commit log for the addition of the TTL decrement action states:
commit f0fd1a1772665ea57662281d9cccadb0f0146196
Author: Pravin B Shelar <pshe...@nicira.com>
Date: Fri Jan 13 17:54:04 2012 -0800
ofproto: New action TTL decrement.
Following patch implements dec_ttl as vendor action with similar
semantics as OpenFlow 1.2. If TTL reaches zero while procession
actions in current table, the remaining actions in previous tables
are processed. A configuration parameter is added to make TTL
decrement to zero generate packet in.
Feature #8758
Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
But the code features "if (ttl > 1)".
That now seems like a logic bug to me (though perhaps the afternoon
air is affecting my brain adversely).
I have revised the dec_mpls_ttl patch to use "(ttl > 0)".
Should I provide a patch to update dec_ttl?
From: Simon Horman <ho...@verge.net.au>
Date: Thu Oct 18 16:42:41 2012 +0900
Add support for dec_mpls_ttl action
This adds support for the OpenFlow 1.1+ dec_mpls_ttl action.
And also adds an NX dec_mpls_ttl action.
The handling of the TTL modification is entirely handled in userspace.
Reviewed-by: Isaku Yamahata <yamah...@valinux.co.jp>
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
v2.21 (draft)
* Allow decrement of TTL.
Previously this was disallowed along with decrementing TTL 0.
v2.18 - v2.20
* No change
v2.17
* Rebase
* As suggested by Ben Pfaff
- Use consistent terminology for MPLS.
+ Consistently refer to the MPLS component of a packet as the
MPLS label stack and entries in the stack as MPLS label stack entries
(LSE). An MPLS label is a component of an MPLS label stack entry.
The other components are the traffic class (TC), time to live (TTL)
and bottom of stack (BoS) bit.
- Rename compose_dec_mpls_ttl_action as execute_dec_mpls_ttl_action
v2.13 - v2.16
* No change
v2.12
* Use eth_type_mpls() helper
v2.11 - v2.10
* No change
v2.9
* Update tests for upstream changes
v2.8
* No change
v2.7
* Encode action as OFP11 action in OFP11+ messages
v2.6
* Rebase
v2.5
* First post
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index c4ff904..db96be9 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,7 @@ enum nx_action_subtype {
NXAST_WRITE_METADATA, /* struct nx_action_write_metadata */
NXAST_PUSH_MPLS, /* struct nx_action_push_mpls */
NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+ NXAST_DEC_MPLS_TTL, /* struct nx_action_header */
};
/* Header for Nicira-defined actions. */
diff --git a/lib/flow.c b/lib/flow.c
index 397bda1..967f268 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -848,6 +848,14 @@ flow_set_mpls_label(struct flow *flow, ovs_be32 label)
set_mpls_lse_label(&flow->mpls_lse, label);
}
+/* Sets the MPLS TTL that 'flow' matches to 'ttl', which should be in the
+ * range 0...255. */
+void
+flow_set_mpls_ttl(struct flow *flow, uint8_t ttl)
+{
+ set_mpls_lse_ttl(&flow->mpls_lse, ttl);
+}
+
/* Sets the MPLS TC that 'flow' matches to 'tc', which should be in the
* range 0...7. */
void
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index d6fc429..2b655dc 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -406,6 +406,10 @@ ofpact_from_nxast(const union ofp_action *a, enum
ofputil_action_code code,
break;
}
+ case OFPUTIL_NXAST_DEC_MPLS_TTL:
+ ofpact_put_DEC_MPLS_TTL(out);
+ break;
+
case OFPUTIL_NXAST_POP_MPLS: {
struct nx_action_pop_mpls *nxapm = (struct nx_action_pop_mpls *)a;
if (eth_type_mpls(nxapm->ethertype)) {
@@ -786,6 +790,10 @@ ofpact_from_openflow11(const union ofp_action *a, struct
ofpbuf *out)
return nxm_reg_load_from_openflow12_set_field(
(const struct ofp12_action_set_field *)a, out);
+ case OFPUTIL_OFPAT11_DEC_MPLS_TTL:
+ ofpact_put_DEC_MPLS_TTL(out);
+ break;
+
case OFPUTIL_OFPAT11_PUSH_MPLS: {
struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
if (!eth_type_mpls(oap->ethertype)) {
@@ -1136,6 +1144,7 @@ ofpact_check__(const struct ofpact *a, const struct flow
*flow, int max_ports,
}
case OFPACT_DEC_TTL:
+ case OFPACT_DEC_MPLS_TTL:
case OFPACT_SET_TUNNEL:
case OFPACT_SET_QUEUE:
case OFPACT_POP_QUEUE:
@@ -1383,6 +1392,10 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf
*out)
ofpact_dec_ttl_to_nxast(ofpact_get_DEC_TTL(a), out);
break;
+ case OFPACT_DEC_MPLS_TTL:
+ ofputil_put_NXAST_DEC_MPLS_TTL(out);
+ break;
+
case OFPACT_SET_TUNNEL:
ofpact_set_tunnel_to_nxast(ofpact_get_SET_TUNNEL(a), out);
break;
@@ -1550,6 +1563,7 @@ ofpact_to_openflow10(const struct ofpact *a, struct
ofpbuf *out)
case OFPACT_REG_MOVE:
case OFPACT_REG_LOAD:
case OFPACT_DEC_TTL:
+ case OFPACT_DEC_MPLS_TTL:
case OFPACT_SET_TUNNEL:
case OFPACT_WRITE_METADATA:
case OFPACT_SET_QUEUE:
@@ -1682,6 +1696,10 @@ ofpact_to_openflow11(const struct ofpact *a, struct
ofpbuf *out)
ofpact_dec_ttl_to_openflow11(ofpact_get_DEC_TTL(a), out);
break;
+ case OFPACT_DEC_MPLS_TTL:
+ ofputil_put_OFPAT11_DEC_MPLS_TTL(out);
+ break;
+
case OFPACT_WRITE_METADATA:
/* OpenFlow 1.1 uses OFPIT_WRITE_METADATA to express this action. */
break;
@@ -1826,6 +1844,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact,
uint16_t port)
case OFPACT_REG_MOVE:
case OFPACT_REG_LOAD:
case OFPACT_DEC_TTL:
+ case OFPACT_DEC_MPLS_TTL:
case OFPACT_SET_TUNNEL:
case OFPACT_WRITE_METADATA:
case OFPACT_SET_QUEUE:
@@ -2048,6 +2067,10 @@ ofpact_format(const struct ofpact *a, struct ds *s)
print_dec_ttl(ofpact_get_DEC_TTL(a), s);
break;
+ case OFPACT_DEC_MPLS_TTL:
+ ds_put_cstr(s, "dec_mpls_ttl");
+ break;
+
case OFPACT_SET_TUNNEL:
tunnel = ofpact_get_SET_TUNNEL(a);
ds_put_format(s, "set_tunnel%s:%#"PRIx64,
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index c4e1b4a..5746a5a 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -71,6 +71,7 @@
DEFINE_OFPACT(REG_MOVE, ofpact_reg_move, ofpact) \
DEFINE_OFPACT(REG_LOAD, ofpact_reg_load, ofpact) \
DEFINE_OFPACT(DEC_TTL, ofpact_cnt_ids, cnt_ids) \
+ DEFINE_OFPACT(DEC_MPLS_TTL, ofpact_null, ofpact) \
DEFINE_OFPACT(PUSH_MPLS, ofpact_push_mpls, ofpact) \
DEFINE_OFPACT(POP_MPLS, ofpact_pop_mpls, ofpact) \
\
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 5fda08a..7b881af 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -550,6 +550,11 @@ parse_named_action(enum ofputil_action_code code, const
struct flow *flow,
parse_dec_ttl(ofpacts, arg);
break;
+ case OFPUTIL_OFPAT11_DEC_MPLS_TTL:
+ case OFPUTIL_NXAST_DEC_MPLS_TTL:
+ ofpact_put_DEC_MPLS_TTL(ofpacts);
+ break;
+
case OFPUTIL_NXAST_FIN_TIMEOUT:
parse_fin_timeout(ofpacts, arg);
break;
diff --git a/lib/ofp-util.def b/lib/ofp-util.def
index ff7e1b3..d55b1eb 100644
--- a/lib/ofp-util.def
+++ b/lib/ofp-util.def
@@ -30,6 +30,7 @@ OFPAT11_ACTION(OFPAT11_SET_NW_TOS, ofp_action_nw_tos, 0,
"mod_nw_tos")
//OFPAT11_ACTION(OFPAT11_SET_NW_ECN, ofp11_action_nw_ecn, "0, mod_nw_ecn")
OFPAT11_ACTION(OFPAT11_SET_TP_SRC, ofp_action_tp_port, 0, "mod_tp_src")
OFPAT11_ACTION(OFPAT11_SET_TP_DST, ofp_action_tp_port, 0, "mod_tp_dst")
+OFPAT11_ACTION(OFPAT11_DEC_MPLS_TTL, ofp_action_header, 0, "dec_mpls_ttl")
OFPAT11_ACTION(OFPAT11_PUSH_VLAN, ofp11_action_push, 0, "push_vlan")
OFPAT11_ACTION(OFPAT11_POP_VLAN, ofp_action_header, 0, "pop_vlan")
OFPAT11_ACTION(OFPAT11_PUSH_MPLS, ofp11_action_push, 0, "push_mpls")
@@ -63,6 +64,7 @@ NXAST_ACTION(NXAST_CONTROLLER, nx_action_controller,
0, "controller")
NXAST_ACTION(NXAST_DEC_TTL_CNT_IDS, nx_action_cnt_ids, 1, NULL)
NXAST_ACTION(NXAST_WRITE_METADATA, nx_action_write_metadata, 0,
"write_metadata")
+NXAST_ACTION(NXAST_DEC_MPLS_TTL, nx_action_header, 0, "dec_mpls_ttl")
NXAST_ACTION(NXAST_PUSH_MPLS, nx_action_push_mpls, 0, "push_mpls")
NXAST_ACTION(NXAST_POP_MPLS, nx_action_pop_mpls, 0, "pop_mpls")
diff --git a/lib/packets.c b/lib/packets.c
index a39cddf..c33ae08 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -259,7 +259,7 @@ static bool is_mpls(struct ofpbuf *packet)
}
/* Set time to live (TTL) of an MPLS label stack entry (LSE). */
-static void
+void
set_mpls_lse_ttl(ovs_be32 *lse, uint8_t ttl)
{
*lse &= ~htonl(MPLS_TTL_MASK);
diff --git a/lib/packets.h b/lib/packets.h
index 0f97fe6..b73ff63 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -155,6 +155,7 @@ void set_mpls_lse(struct ofpbuf *, ovs_be32 label);
void push_mpls(struct ofpbuf *packet, ovs_be16 ethtype, ovs_be32 lse);
void pop_mpls(struct ofpbuf *, ovs_be16 ethtype);
+void set_mpls_lse_ttl(ovs_be32 *lse, uint8_t ttl);
void set_mpls_lse_tc(ovs_be32 *lse, uint8_t tc);
void set_mpls_lse_label(ovs_be32 *lse, ovs_be32 label);
void set_mpls_lse_bos(ovs_be32 *lse, uint8_t bos);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a4d7e59..e8d11a6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6098,6 +6098,27 @@ compose_dec_ttl(struct action_xlate_ctx *ctx, struct
ofpact_cnt_ids *ids)
}
}
+static bool
+execute_dec_mpls_ttl_action(struct action_xlate_ctx *ctx)
+{
+ uint8_t ttl = mpls_lse_to_ttl(ctx->flow.mpls_lse);
+
+ if (!eth_type_mpls(ctx->flow.dl_type)) {
+ return false;
+ }
+
+ if (ttl > 0) {
+ ttl--;
+ set_mpls_lse_ttl(&ctx->flow.mpls_lse, ttl);
+ return false;
+ } else {
+ execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
+
+ /* Stop processing for current table. */
+ return true;
+ }
+}
+
static void
xlate_output_action(struct action_xlate_ctx *ctx,
uint16_t port, uint16_t max_len, bool may_packet_in)
@@ -6441,6 +6462,12 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
execute_mpls_pop_action(ctx, ofpact_get_POP_MPLS(a)->ethertype);
break;
+ case OFPACT_DEC_MPLS_TTL:
+ if (execute_dec_mpls_ttl_action(ctx)) {
+ goto out;
+ }
+ break;
+
case OFPACT_DEC_TTL:
if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) {
goto out;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 8f11b82..cd08e29 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -233,6 +233,7 @@ cookie=0x9 table=7 in_port=86
actions=mod_tp_dst:86,controller,controller
cookie=0xa dl_src=40:44:44:44:44:42
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
cookie=0xa dl_src=40:44:44:44:44:43
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
cookie=0xa dl_src=40:44:44:44:44:44
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
+cookie=0xa dl_src=40:44:44:44:44:45
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,controller
cookie=0xb dl_src=50:55:55:55:55:55 dl_type=0x8847
actions=load:1000->OXM_OF_MPLS_LABEL[[]],controller
cookie=0xd dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,controller
cookie=0xc dl_src=70:77:77:77:77:77
actions=push_mpls:0x8848,load:1000->OXM_OF_MPLS_LABEL[[]],load:7->OXM_OF_MPLS_TC[[]],controller
@@ -355,6 +356,25 @@ NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1
(via action) data_len
mpls(label:10,tc:3,ttl:64,bos:1),metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=7,dl_src=40:44:44:44:44:44,dl_dst=50:54:00:00:00:07
])
+dnl Modified MPLS controller action.
+AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2>
ofctl_monitor.log])
+
+for i in 1 2 3; do
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),eth(src=40:44:44:44:44:45,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action)
data_len=64 (unbuffered)
+mpls(label:10,tc:3,ttl:63,bos:1),metadata=0,in_port=0,vlan_tci=0x0000,dl_src=40:44:44:44:44:45,dl_dst=50:54:00:00:00:07
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action)
data_len=64 (unbuffered)
+mpls(label:10,tc:3,ttl:63,bos:1),metadata=0,in_port=0,vlan_tci=0x0000,dl_src=40:44:44:44:44:45,dl_dst=50:54:00:00:00:07
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action)
data_len=64 (unbuffered)
+mpls(label:10,tc:3,ttl:63,bos:1),metadata=0,in_port=0,vlan_tci=0x0000,dl_src=40:44:44:44:44:45,dl_dst=50:54:00:00:00:07
+])
+
dnl Modified MPLS actions.
AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2>
ofctl_monitor.log])
@@ -500,6 +520,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort],
[0], [dnl
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:42
actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:43
actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:44
actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:45
actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,CONTROLLER:65535
cookie=0xb, n_packets=3, n_bytes=180, dl_src=50:55:55:55:55:55,dl_type=0x8847
actions=load:0x3e8->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
cookie=0xc, n_packets=3, n_bytes=180, dl_src=70:77:77:77:77:77
actions=push_mpls:0x8848,load:0x3e8->OXM_OF_MPLS_LABEL[[]],load:0x7->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
cookie=0xd, n_packets=3, n_bytes=180, dl_src=60:66:66:66:66:66
actions=pop_mpls:0x0800,CONTROLLER:65535
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 6589eff..40869c4 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1018,6 +1018,15 @@ invalid ttl packets. If controller ids are not
specified, the
``packet_in'' message will be sent only to the controllers having
controller id zero which have registered for the invalid ttl packets.
.
+.IP \fBdec_mpls_ttl\fR
+Decrement TTL of the outer MPLS label stack entry of a packet. If the TTL
+is initially zero, no decrement occurs. Instead, a ``packet-in'' message
+with reason code \fBOFPR_INVALID_TTL\fR is sent to each connected
+controller with controller id zer that has enabled receiving them.
+Processing the current set of actions then stops. However, if the current
+set of actions was reached through ``resubmit'' then remaining actions in
+outer levels resume processing.
+.
.IP \fBnote:\fR[\fIhh\fR]...
Does nothing at all. Any number of bytes represented as hex digits
\fIhh\fR may be included. Pairs of hex digits may be separated by
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
