[ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header Support

Yang, Yi yi.y.yang at intel.com
Fri Jul 15 02:26:40 UTC 2016


On Thu, Jul 14, 2016 at 07:45:06AM -0700, Jesse Gross wrote:
> >
> > Currently, struct tun_metadata in struct flow_tnl.
> >
> > /* Tunnel information used in flow key and metadata. */
> > struct flow_tnl {
> >     ovs_be32 ip_dst;
> >     struct in6_addr ipv6_dst;
> >     ovs_be32 ip_src;
> >     struct in6_addr ipv6_src;
> >     ovs_be64 tun_id;
> >     uint16_t flags;
> >     uint8_t ip_tos;
> >     uint8_t ip_ttl;
> >     ovs_be16 tp_src;
> >     ovs_be16 tp_dst;
> >     ovs_be16 gbp_id;
> >     uint8_t  gbp_flags;
> >     uint8_t  pad1[5];        /* Pad to 64 bits. */
> >     struct tun_metadata metadata;
> > };
> >
> > But NSH isn't a tunnel, so NSH metadata shouldn't be here, does it make
> > sense?
> >
> > We propose to move "struct tun_metadata metadata;" to struct flow, this
> > won't increase size of struct flow. In this way, we can define a union
> > in struct flow
> >
> > union {
> >     struct metadata tun_metadata;
> >     struct metadata nsh_metadata;
> > } u;
> >
> > Geneve should operate flow.u.tun_metadata, NSH should operate
> > flow.u.nsh_metadata, make sense?
> 
> I think that sounds fine as long as you are careful - one particular
> thing to pay attention will be DPDK performance. DPDK tends to be very
> sensitive to the size of the flow key and storing the metadata in the
> tunnel area is part of an optimization to avoid initializing memory
> when not necessary. We'll need to make sure that these changes don't
> cause any regression in that area.

Got it, we'll do this carefully and check any possible performance
regression.



More information about the dev mailing list