On Wed, Mar 1, 2017 at 2:47 PM, Eric Garver <e...@erig.me> wrote: > Flow key handling changes: > - Add VLAN header array in struct flow, to record multiple 802.1q VLAN > headers. > - Add dpif multi-VLAN capability probing. If datapath supports > multi-VLAN, increase the maximum depth of nested OVS_KEY_ATTR_ENCAP. > > Refactor VLAN handling in dpif-xlate: > - Introduce 'xvlan' to track VLAN stack during flow processing. > - Input and output VLAN translation according to the xbundle type. > > Push VLAN action support: > - Allow ethertype 0x88a8 in VLAN headers and push_vlan action. > - Support push_vlan on dot1q packets. > > Use other_config:vlan-limit in table Open_vSwitch to limit maximum VLANs > that can be matched. This allows us to preserve backwards compatibility. > > Add test cases for VLAN depth limit, Multi-VLAN actions and QinQ VLAN > handling > > Co-authored-by: Thomas F Herbert <thomasfherb...@gmail.com> > Signed-off-by: Thomas F Herbert <thomasfherb...@gmail.com> > Co-authored-by: Xiao Liang <shaw.l...@gmail.com> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com> > Signed-off-by: Eric Garver <e...@erig.me>
Thanks for working on this. And sorry it took a while to get those reviewed. The patch does not apply cleanly to master. Do you mind rebase and repost? I suppose you will need to this anyways for them to be pushed. I have a few comments for this patch, I will review this other patches more carefully after rebase. git am complained about extra while space. I think it is about the line below ofproto_set_vlan_limit() > > v2.7.0 - xx xxx xxxx > --------------------- > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h > index df80dfe46199..0ed02e08fd2d 100644 > --- a/include/openvswitch/flow.h > +++ b/include/openvswitch/flow.h > @@ -23,7 +23,7 @@ > /* This sequence number should be incremented whenever anything involving > flows > * or the wildcarding of flows changes. This will cause build assertion > * failures in places which likely need to be updated. */ > -#define FLOW_WC_SEQ 36 > +#define FLOW_WC_SEQ 37 > > /* Number of Open vSwitch extension 32-bit registers. */ > #define FLOW_N_REGS 16 > @@ -61,6 +61,12 @@ const char *flow_tun_flag_to_string(uint32_t flags); > /* Maximum number of supported MPLS labels. */ > #define FLOW_MAX_MPLS_LABELS 3 > > +/* Maximum number of supported VLAN headers. */ > +#define FLOW_MAX_VLAN_HEADERS 2 > + > +/* Legacy maximum VLAN headers */ > +#define LEGACY_MAX_VLAN_HEADERS 1 > + > /* > * A flow in the network. > * > @@ -103,7 +109,8 @@ struct flow { > struct eth_addr dl_dst; /* Ethernet destination address. */ > struct eth_addr dl_src; /* Ethernet source address. */ > ovs_be16 dl_type; /* Ethernet frame type. */ > - ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */ > + uint8_t pad2[2]; /* Pad to 64 bits. */ Do we need 4 more bytes for 64 bit alignment? > +void > +flow_pop_vlan(struct flow *flow, struct flow_wildcards *wc) > +{ > + int n = flow_count_vlan_headers(flow); > + if (n == 0) { > + return; > + } Can n be 0? or should this be an assert instead? > + if (wc) { > + memset(&wc->masks.vlans[1], 0xff, > + sizeof(union flow_vlan_hdr) * (n - 1)); > + } Should we also set the mask for wc->masks.vlans[0]? > + memmove(&flow->vlans[0], &flow->vlans[1], > + sizeof(union flow_vlan_hdr) * (n - 1)); > + memset(&flow->vlans[n - 1], 0, sizeof(union flow_vlan_hdr)); > +} > + > +void > +flow_push_vlan_uninit(struct flow *flow, struct flow_wildcards *wc) > +{ > + int n = flow_count_vlan_headers(flow); Do we always know there is room in the flow? > + if (wc) { > + memset(wc->masks.vlans, 0xff, sizeof(union flow_vlan_hdr) * n); > + } Should we set mask for the vlan fields that is about to be pushed? > + memmove(&flow->vlans[1], &flow->vlans[0], > + sizeof(union flow_vlan_hdr) * (FLOW_MAX_VLAN_HEADERS - 1)); > + memset(&flow->vlans[0], 0, sizeof(union flow_vlan_hdr)); The function syas "_uninit", I don't think the last memset() is needed. > } > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev