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

Reply via email to