On Tue, Jan 13, 2015 at 3:32 AM, Thomas Graf <tg...@suug.ch> wrote: > On 01/12/15 at 06:28pm, Tom Herbert wrote: >> On Mon, Jan 12, 2015 at 5:03 PM, Thomas Graf <tg...@suug.ch> wrote: >> >> >> >> Creating a level of indirection for extensions seems overly >> >> complicated to me. Why not just define IFLA_VXLAN_GBP as just another >> >> enum above? >> > >> > I think it's cleaner to group them in a nested attribute. >> > It clearly separates the optional extensions from the base >> > attributes. RCO, GPE, GBP can all live in there. >> >> This is inconsistent with similar things in GRE and GUE. For instance, >> GRE keyid is set as its own attribute. It just seems like this adding >> more code to the driver than is necessary for the functionality >> needed. > > The major difference here is that we have to consider backwards > compatibility specifically for VXLAN. Your initial feedback on GPE > actually led me to how I implemented GBP. > > I think the axioms we want to establish are as follows: > 1. Extensions need to be explicitly enabled by the user. A previously > dropped frame should only be processed if the user explitly asks > for it. > 2. As a consequence: only share a VLXAN UDP port if the enabled > extensions match (vxlan_sock_add), e.g. user A might want RCO > but user B might be unaware. They cannot share the same UDP port. > > The 2nd lead me to introduce the 'exts' member to vxlan_sock so we can > compare it in vxlan_find_sock() and only share a UDP port if the > enabled extensions match. > RCO is represented in the socket in VXLAN flags (VLXAN_F_*). My patch also adds a flags to vxlan_sock which contains the VLXAN flags. For shared port, I suspect all the receive features must match, including receive checksum settings for instance, but we don't care about transmit side. To facilitate this, I would suggest splitting flags into o_flags and i_flags like ip_tunnel does, and then compare i_flags in vxlan_find_sock.
Regardless of the internal implementation, I still don't see much value in exposing these distinctions in netlink. Tom > Your patch currently implements (1) but not (2). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
