On Mon, Sep 10, 2018 at 03:03:19AM +0000, Lilijun (Jerry, Cloud Networking) wrote: > Hi Eric, > > Yes, I agree with that effect. > But how about this issue of QinQ that we can only support at most 2 VLANs ? > Do you have any ideas?
I was not NACKing the idea. Just wanted everyone to understand the implications of increasing the VLAN field size. I tried playing with the fields, but didn't come with a reasonable way to rearrange them to make room for the extra VLANs. I'm curious what you're use case is for triple VLAN. I wonder if VXLAN or PBB (802.1ah) is a better solution. > > Thanks. > > -----Original Message----- > From: Eric Garver [mailto:[email protected]] > Sent: Friday, September 07, 2018 10:14 PM > To: Lilijun (Jerry, Cloud Networking) <[email protected]> > Cc: Ben Pfaff <[email protected]>; [email protected]; > [email protected] > Subject: Re: [ovs-discuss] [PATCH v2] [ovs-dev] [PATCH] QinQ: support more > vlan headers. > > On Fri, Sep 07, 2018 at 09:53:44AM +0000, Lilijun (Jerry, Cloud Networking) > wrote: > > In my test, vlan-limit is set to 0 that means unlimited the count of > > vlan headers. > > ovs-vsctl set Open_vSwitch . other_config:vlan-limit=0 > > > > But in fact the macro FLOW_MAX_VLAN_HEADERS is defined as 2, so we can > > only support max two vlan headers. It doesn't work as the config > > vlan-limit's description. > > > > So, when VM sends a packet already with two vlan headers, the vport > > configured with qinq mode can't add another s-vlan headers. > > FLOW_MAX_VLAN_HEADERS need to be larger to support more vlan headers. > > > > Change-Id: I8449e308d406ce3757b43a2636ff0f326ca12a9d > > Signed-off-by: Lilijun <[email protected]> > > --- > > include/openvswitch/flow.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h > > index 5d2cf09..a09650c 100644 > > --- a/include/openvswitch/flow.h > > +++ b/include/openvswitch/flow.h > > @@ -72,7 +72,7 @@ const char *flow_tun_flag_to_string(uint32_t flags); > > * > > * We require this to be a multiple of 2 so that vlans[] in struct flow is > > a > > * multiple of 64 bits. */ > > -#define FLOW_MAX_VLAN_HEADERS 2 > > +#define FLOW_MAX_VLAN_HEADERS 4 > > This change splits the MPLS labels and ct_ipv6_src across cache lines and 64 > byte boundary which affects the miniflow. This change may have a performance > impact for scenarios that use these fields. > > On the plus side it pushes all the nsh fields into the same cache line. > Previously they were split. > > With this patch: > > $ pahole -C flow lib/flow.o > struct flow { > struct flow_tnl tunnel; /* 0 344 */ > /* --- cacheline 5 boundary (320 bytes) was 24 bytes ago --- */ > ovs_be64 metadata; /* 344 8 */ > uint32_t regs[16]; /* 352 64 */ > /* --- cacheline 6 boundary (384 bytes) was 32 bytes ago --- */ > uint32_t skb_priority; /* 416 4 */ > uint32_t pkt_mark; /* 420 4 */ > uint32_t dp_hash; /* 424 4 */ > union flow_in_port in_port; /* 428 4 */ > uint32_t recirc_id; /* 432 4 */ > uint8_t ct_state; /* 436 1 */ > uint8_t ct_nw_proto; /* 437 1 */ > uint16_t ct_zone; /* 438 2 */ > uint32_t ct_mark; /* 440 4 */ > ovs_be32 packet_type; /* 444 4 */ > /* --- cacheline 7 boundary (448 bytes) --- */ > ovs_u128 ct_label; /* 448 16 */ > uint32_t conj_id; /* 464 4 */ > ofp_port_t actset_output; /* 468 4 */ > struct eth_addr dl_dst; /* 472 6 */ > struct eth_addr dl_src; /* 478 6 */ > ovs_be16 dl_type; /* 484 2 */ > uint8_t pad1[2]; /* 486 2 */ > union flow_vlan_hdr vlans[4]; /* 488 16 */ > ovs_be32 mpls_lse[4]; /* 504 16 */ > /* --- cacheline 8 boundary (512 bytes) was 8 bytes ago --- */ > ovs_be32 nw_src; /* 520 4 */ > ovs_be32 nw_dst; /* 524 4 */ > ovs_be32 ct_nw_src; /* 528 4 */ > ovs_be32 ct_nw_dst; /* 532 4 */ > struct in6_addr ipv6_src; /* 536 16 */ > struct in6_addr ipv6_dst; /* 552 16 */ > struct in6_addr ct_ipv6_src; /* 568 16 */ > /* --- cacheline 9 boundary (576 bytes) was 8 bytes ago --- */ > struct in6_addr ct_ipv6_dst; /* 584 16 */ > ovs_be32 ipv6_label; /* 600 4 */ > uint8_t nw_frag; /* 604 1 */ > uint8_t nw_tos; /* 605 1 */ > uint8_t nw_ttl; /* 606 1 */ > uint8_t nw_proto; /* 607 1 */ > struct in6_addr nd_target; /* 608 16 */ > struct eth_addr arp_sha; /* 624 6 */ > struct eth_addr arp_tha; /* 630 6 */ > ovs_be16 tcp_flags; /* 636 2 */ > ovs_be16 pad2; /* 638 2 */ > /* --- cacheline 10 boundary (640 bytes) --- */ > struct ovs_key_nsh nsh; /* 640 24 */ > ovs_be16 tp_src; /* 664 2 */ > ovs_be16 tp_dst; /* 666 2 */ > ovs_be16 ct_tp_src; /* 668 2 */ > ovs_be16 ct_tp_dst; /* 670 2 */ > ovs_be32 igmp_group_ip4; /* 672 4 */ > ovs_be32 pad3; /* 676 4 */ > > /* size: 680, cachelines: 11, members: 47 */ > /* last cacheline: 40 bytes */ > }; > > > Before this patch: > > $ pahole -C flow lib/flow.o > struct flow { > struct flow_tnl tunnel; /* 0 344 */ > /* --- cacheline 5 boundary (320 bytes) was 24 bytes ago --- */ > ovs_be64 metadata; /* 344 8 */ > uint32_t regs[16]; /* 352 64 */ > /* --- cacheline 6 boundary (384 bytes) was 32 bytes ago --- */ > uint32_t skb_priority; /* 416 4 */ > uint32_t pkt_mark; /* 420 4 */ > uint32_t dp_hash; /* 424 4 */ > union flow_in_port in_port; /* 428 4 */ > uint32_t recirc_id; /* 432 4 */ > uint8_t ct_state; /* 436 1 */ > uint8_t ct_nw_proto; /* 437 1 */ > uint16_t ct_zone; /* 438 2 */ > uint32_t ct_mark; /* 440 4 */ > ovs_be32 packet_type; /* 444 4 */ > /* --- cacheline 7 boundary (448 bytes) --- */ > ovs_u128 ct_label; /* 448 16 */ > uint32_t conj_id; /* 464 4 */ > ofp_port_t actset_output; /* 468 4 */ > struct eth_addr dl_dst; /* 472 6 */ > struct eth_addr dl_src; /* 478 6 */ > ovs_be16 dl_type; /* 484 2 */ > uint8_t pad1[2]; /* 486 2 */ > union flow_vlan_hdr vlans[2]; /* 488 8 */ > ovs_be32 mpls_lse[4]; /* 496 16 */ > /* --- cacheline 8 boundary (512 bytes) --- */ > ovs_be32 nw_src; /* 512 4 */ > ovs_be32 nw_dst; /* 516 4 */ > ovs_be32 ct_nw_src; /* 520 4 */ > ovs_be32 ct_nw_dst; /* 524 4 */ > struct in6_addr ipv6_src; /* 528 16 */ > struct in6_addr ipv6_dst; /* 544 16 */ > struct in6_addr ct_ipv6_src; /* 560 16 */ > /* --- cacheline 9 boundary (576 bytes) --- */ > struct in6_addr ct_ipv6_dst; /* 576 16 */ > ovs_be32 ipv6_label; /* 592 4 */ > uint8_t nw_frag; /* 596 1 */ > uint8_t nw_tos; /* 597 1 */ > uint8_t nw_ttl; /* 598 1 */ > uint8_t nw_proto; /* 599 1 */ > struct in6_addr nd_target; /* 600 16 */ > struct eth_addr arp_sha; /* 616 6 */ > struct eth_addr arp_tha; /* 622 6 */ > ovs_be16 tcp_flags; /* 628 2 */ > ovs_be16 pad2; /* 630 2 */ > struct ovs_key_nsh nsh; /* 632 24 */ > /* --- cacheline 10 boundary (640 bytes) was 16 bytes ago --- */ > ovs_be16 tp_src; /* 656 2 */ > ovs_be16 tp_dst; /* 658 2 */ > ovs_be16 ct_tp_src; /* 660 2 */ > ovs_be16 ct_tp_dst; /* 662 2 */ > ovs_be32 igmp_group_ip4; /* 664 4 */ > ovs_be32 pad3; /* 668 4 */ > > /* size: 672, cachelines: 11, members: 47 */ > /* last cacheline: 32 bytes */ > }; > _______________________________________________ > discuss mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-discuss _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
