On Sat, 27 Feb 2016 12:54:52 -0800, Tom Herbert wrote: > Yes, but RCO has not been specified for VXLAN-GPE either
As far as I can see, RCO will just work with VXLAN-GPE. But I have no problem disallowing them to be set together, if you prefer that. > so the patch > does not correctly refuse setting those two together. Inevitably > though, those and other extensions will defined for VXLAN-GPE and new > ones for VXLAN. Again, the protocols are fundamentally incompatible, > so instead of trying to enforce each valid combination at > configuration We need to do the checking in either case. If we accepted unsupported combinations and then just silently ignored them, we'd be in troubles later when such combination becomes defined/supported. There would be no way for the userspace tools to detect whether a particular kernel supports the combination or not. So, we need to check for supported combination of options during configuration anyway. And when we have that, I don't really see the reason for doing that kind of code duplication that you suggest. > or performing multiple checks for flavor each time we > look at a packet, it seems easier to split the parsing with at most > one check for the protocol variant. For instance in > vxlan_udp_encap_recv just do: > > if (vs->flags & VXLAN_F_GPE) > if (!vxlan_parse_gpe_hdr(&unparsed, skb, vs->flags)) > goto drop; > else > if (!vxlan_parse_gpe(&unparsed, skb, vs->flags)) > goto drop; Most of the code of these two functions will be identical. To consolidate that as much as possible, you'll end up with what I have or something very similar. > And then move REMCSUM and GPB and other protocol specific checks to > the right function. And when RCO is defined for GPE, we copy the code? Doesn't make sense, sorry. If you look at the code in the current net-next (and the code after this patchset), the extension handling has been made generic and each extension gets its own handler function, leading to clean separation in the code. There's no reason to split the vxlan_rcv into two functions doing the same things but with slightly different calls to extensions. Jiri
