[ovs-discuss] [PATCH v2] [ovs-dev] [PATCH] QinQ: support more vlan headers.
Eric Garver
eric at garver.life
Mon Sep 10 19:15:21 UTC 2018
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:eric at garver.life]
> Sent: Friday, September 07, 2018 10:14 PM
> To: Lilijun (Jerry, Cloud Networking) <jerry.lilijun at huawei.com>
> Cc: Ben Pfaff <blp at ovn.org>; dev at openvswitch.org; ovs-discuss at openvswitch.org
> 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 <jerry.lilijun at huawei.com>
> > ---
> > 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
> discuss at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
More information about the discuss
mailing list