On Wed, Jul 13, 2016 at 07:22:39PM -0700, Jesse Gross wrote:
> >>
> >> In any case, I don't think this is a fundamental issue, just a matter
> >> of timing. Since the premise of the original question was that MD type
> >> 2 shouldn't be too much additional work and the series is currently
> >> dependent on Simon's patches, it seems like now might be a good time
> >> for the authors to look into implementing this.
> >
> > Jesse, MD type 2 support needs a lot of changes, you know
> > tunnel_metadata GENEVE is part of tunnel, but it should be part of NSH in
> > NSH,
> > you suggested we are to leverage GENEVE tunnel_metadata, but how to
> > leverage it better is a big question mark for us. they should be called
> > as nsh_metadata in NSH, so we can't directly use GENEVE's match fields
> > and struct flow_tnl in struct flow, we find a better way to reuse Geneve
> > tunnel_metadata code, that needs to generalize Geneve tunnel_metadata
> > code in order that NSH and Geneve can share them but we can have
> > different match fields name but can use a struct metadata in struct
> > flow, what do you think about it?
> >
> > If you agree this, then I can confirm we will have a lot of changes,
> > especially for GENEVE.
>
> It's possible to have aliases for field names in OVS, so that might be
> one possible solution. However, you had the opportunity (and did!) to
> rename these fields or otherwise make them more amenable to your needs
> so I'm not really all that receptive to complaints in this area. In
> any case, I don't think that it is really a good idea to have 2x 256
> bytes in struct flow (which is an internal structure and can still be
> changed) if that is what you are proposing.
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?
>
> > So my personal opinion is to merge MD type 1 support at first, MD type 2
> > needs a lot of changes, most of them are in Geneve side but not in NSH
> > side.
>
> I think I've been pretty consistent about my desire to see both MD
> types contained in the same patch series as well as the constraints
> imposed by the current interfaces (this was also nicely summarized by
> Jan Scheurich). If you need to generalize existing infrastructure
> please do that (but without breaking any existing external
> interfaces).
Ok, we'll implement MD type 2 in next version.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
