On Tue, Jun 10, 2014 at 07:03:44PM +0530, Avinash wrote:
> This commit contains the following changes:
> 1. New vlan variables (tpid and count) are added to flow structure to support
> provider VLANs.
> 2. miniflow_extract() initialises these new variables based on received
> packet.
> 3. Altered flow sequence is modified in all the required places.
> 4. commit_vlan_action() is modified to support stacked VLANs
>
> Signed-off-by: Avinash <avinashand...@gmail.com>
...
> @@ -98,6 +98,8 @@ struct flow {
> uint8_t dl_src[6]; /* Ethernet source address. */
> ovs_be16 dl_type; /* Ethernet frame type. */
> ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
> + ovs_be16 vlan_tpid; /* VLAN identifier; Either 802.1Q or 802.1AD
> */
> + uint8_t vlan_count; /* Number of stacked VLANs */
I spent some time trying to implement MPLS in a similar way, using a
count of the number of stacked MPLS tags. I found in the end that it
was too difficult to understand when bitwise matching came into the
picture. Thus, I'd encourage you to do something similar to how MPLS
label matching works, with an array of TCIs and TPIDs. Obviously it
can't be exactly the same since VLAN and MPLS are different.
How do you plan to make this work with OpenFlow?
> if (is_mask
> ? (src_flow->vlan_tci & htons(VLAN_CFI)) != 0
> - : src_flow->dl_type == htons(ETH_TYPE_VLAN)) {
Please don't introduce unnecessary parentheses like this (this is
covered in CodingStyle I believe):
> + : (eth_type_vlan(src_flow->dl_type))) {
Ditto here:
- if (base->vlan_tci == vlan_tci) {
+ if ((base->vlan_tci == flow->vlan_tci) &&
+ (base->vlan_count == flow->vlan_count)) {
and here:
if (veh && ofpbuf_size(packet) >= sizeof *veh
- && veh->veth_type == htons(ETH_TYPE_VLAN)) {
+ && (eth_type_vlan(veh->veth_type))) {
Also please see CodingStyle for an example of proper style here:
+ return (vlan_count >= 2)
+ ? htons(ETH_TYPE_VLAN_8021AD)
+ : htons(ETH_TYPE_VLAN);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
