I made the following comment on v1, did you do anything about it?

> Also, the action parser in ofp-actions.def rejects any pop_mpls action
> with an MPLS ethertype attached, in two places.  We should fix that.

Thanks,

Ben.

On Thu, May 02, 2013 at 01:26:38PM +0900, Simon Horman wrote:
> The ethertype should always be updated on mpls_pop
> as there may be a transition between MPLS unicast (0x8847) and
> MPLS multicast (0x8848).
> 
> Ben Pfaff tells me that this is consistent with the
> behaviour described in EXT-194 of the JIRA bug tracker.
> 
> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
> 
> ---
> 
> v2
> * Add note about EXT-194 to changelog
> * Update manpage. Note that I plan to post a patch to
>   make use of recirculation to remove restrictions on
>   the use of MPLS pop and push.
> ---
>  lib/packets.c            | 3 +--
>  utilities/ovs-ofctl.8.in | 7 ++++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/packets.c b/lib/packets.c
> index 77aa7d3..f2e0f81 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -373,9 +373,8 @@ pop_mpls(struct ofpbuf *packet, ovs_be16 ethtype)
>          size_t len;
>          mh = packet->l2_5;
>          len = (char*)packet->l2_5 - (char*)packet->l2;
> -        /* If bottom of the stack set ethertype. */
> +        set_ethertype(packet, ethtype);
>          if (mh->mpls_lse & htonl(MPLS_BOS_MASK)) {
> -            set_ethertype(packet, ethtype);
>              packet->l2_5 = NULL;
>          } else {
>              packet->l2_5 = (char*)packet->l2_5 + MPLS_HLEN;
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index f46b9dc..d316987 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -933,9 +933,10 @@ followed by another \fBpush_mpls\fR will result in the 
> first
>  \fBpush_mpls\fR being discarded.
>  .
>  .IP \fBpop_mpls\fR:\fIethertype\fR
> -Strips the outermost MPLS label stack entry.  If the MPLS label
> -stripped was the only one, changes the ethertype of a packet to
> -\fIethertype\fR, which should not ordinarily be an MPLS Ethertype.
> +Strips the outermost MPLS label stack entry.
> +Currently the implementation restricts \fIethertype\fR to a non-MPLS 
> Ethertype
> +and thus \fBpop_mpls\fR should only be applied to packets with
> +an MPLS label stack depth of one.
>  .
>  .IP
>  There are some limitations in the implementation.  \fBpop_mpls\fR
> -- 
> 1.8.2.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to