You're quite right. I think for OF1.2, this is similar to existing behaviour, but for OF1.3 it's just incorrect. There is an additional issue with the LOAD action.
OF1.2: "push_vlan(A),push_mpls,push_vlan(B)" In OF1.2, this has the same result as:- "push_mpls,push_vlan(A),push_vlan(B)" When translated, this boils down to "(pop_vlan,)push_mpls,push_vlan(B)". Correct me if I am wrong, but I think this is similar to the existing behaviour, as we currently only support one layer of VLAN. It doesn't matter if A == B. OF1.3: "mod_vlan_vid:A,push_mpls:0x8847,mod_vlan_vid:A" should work, but the second mod_vlan is being dropped as it has the same VID as the first. This is incorrect, as you point out. LOAD: "load:A->OXM_OF_VLAN_VID,push_mpls:0x8847,load:A->OXM_OF_VLAN_VID" doesn't result in the correct vlan_vid from the first load action. I'm not sure that vlan_tci_restore() is clear or correct---I believe its original purpose was to properly handle the case where MPLS changes are made from REG_LOAD and friends, in the way that the PUSH_MPLS case works. Instead, it is handling all cases where MPLS actions have been applied in the past, whether the current action modifies MPLS or not. It's probably also worthwhile to make use of the ovs-ofctl monitor "-m" option in the tests to actually verify these, rather than the current tests where we just check the size of the resulting packet.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev