Hi Xiao, Thanks for the series. One patch comment below.
General comments: 1) Various places use VLAN_CFI to indicate presence of a VLAN tag (many checks for tci != 0, because CFI bit set). Since we're now tracking the TPID would it be more appropriate to instead use the TPID to indicate presence of a VLAN tag? 2) eth_type_vlan() checks only 0x8100 and 0x88a8. Older switch implementations used 0x9100 for outer tags. It may be worth supporting 0x9100 as well even though OF v1.1 only specifies 0x8100 and 0x88a8. Thanks. Eric. On Sun, Jul 03, 2016 at 08:47:06AM +0800, Xiao Liang wrote: > diff --git a/tests/test-classifier.c b/tests/test-classifier.c > index c74c440..1826936 100644 > --- a/tests/test-classifier.c > +++ b/tests/test-classifier.c > @@ -1702,7 +1702,8 @@ test_miniflow(struct ovs_cmdl_context *ctx OVS_UNUSED) > miniflow = miniflow_create(&flow); > > /* Check that the flow equals its miniflow. */ > - assert(miniflow_get_vid(miniflow) == vlan_tci_to_vid(flow.vlan_tci)); > + assert(miniflow_get_vid(miniflow) == > + vlan_tci_to_vid(flow.vlan[0].tci)); > for (i = 0; i < FLOW_U64S; i++) { > assert(miniflow_get(miniflow, i) == flow_u64[i]); > } This only checks the outer VLAN. Should it also check the inner? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev