On 02/16/13 00:45, Jesse Gross wrote: > On Fri, Feb 15, 2013 at 10:34 AM, Lori Jakab <loja...@cisco.com> wrote: >> On 02/15/13 01:32, Jesse Gross wrote: >>> On Wed, Feb 13, 2013 at 6:44 AM, Lorand Jakab <loja...@cisco.com> wrote: >>>> diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c >>>> new file mode 100644 >>>> index 0000000..9ca851c >>>> --- /dev/null >>>> +++ b/datapath/vport-lisp.c >>> [...] >>>> +#define LISP_DST_PORT 4341 /* Well known UDP port for LISP data packets. >>>> */ >>> >>> Does it make sense to make this configurable as it is for VXLAN? I >>> realize that unlike VXLAN, IANA has allocated a port for LISP. >>> However, perhaps an analogous situation is whether one would want to >>> bake the port number into an ASIC. >> >> Configurability would only be useful between OVS implementations, since >> all other implementations to date that I know of have this IANA >> allocated port number hardcoded. It helps network monitoring as well, >> if LISP traffic is using the well known port only. The only benefit of >> configurable UDP ports I can think of would be to avoid restrictive >> firewalls (set it to port 53?). So personally I don't see value in >> making this configurable, but I'm open to be convinced. ;) > > We don't have to make it configurable all the way to the user but it > seems like exposing it to userspace makes sense given that we already > have the infrastructure from VXLAN. In many ways, I view the kernel > module as the equivalent of a forwarding ASIC in a physical switch, > which is why I asked the question about whether it would be hardcoded > in the ASIC. My feeling is that usually it would not be since L4 > ports definitely tend to be more fluid for all kinds of reasons (it > the case that you mentioned, it's likely that your DNS server allows > you to specify the port to listen on even though it is very well > known).
OK, I will add support for this a-la-VXLAN. > >>>> +/* Compute source port for outgoing packet. >>>> + * Currently we use the flow hash. >>>> + */ >>>> +static u16 get_src_port(struct sk_buff *skb) >>>> +{ >>>> + int low; >>>> + int high; >>>> + unsigned int range; >>>> + u32 hash = OVS_CB(skb)->flow->hash; >>>> + >>>> + inet_get_local_port_range(&low, &high); >>>> + range = (high - low) + 1; >>>> + return (((u64) hash * range) >> 32) + low; >>>> +} >>> >>> Since this is the same as VXLAN, it might be worth factoring it out to >>> some common location. >> >> What would be the correct place for this? datapath/tunnel.c ? Or maybe >> as a 'static inline' function in datapath/tunnel.h ? I will move it >> there and remove it from the VXLAN vport code as well. > > Probably datapath/tunnel.h makes the most sense. OK. There is one issue with this though. datapath/datapath.h includes datapath/tunnel.h, but the OVS_CB macro, required by the the new function, is defined in the former. I can either expand the macro in place of OVS_CB in tunnel.h (a bit ugly) or move the #define from datapath.h to tunnel.h. Which solution would you prefer? > >>>> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h >>>> index 7c6e3ab..edccf11 100644 >>>> --- a/include/linux/openvswitch.h >>>> +++ b/include/linux/openvswitch.h >>>> @@ -184,6 +184,7 @@ enum ovs_vport_type { >>>> OVS_VPORT_TYPE_INTERNAL, /* network device implemented by datapath >>>> */ >>>> OVS_VPORT_TYPE_GRE, /* GRE tunnel. */ >>>> OVS_VPORT_TYPE_VXLAN, /* VXLAN tunnel */ >>>> + OVS_VPORT_TYPE_LISP, /* LISP tunnel */ >>> >>> Assuming that this continues to evolve to move natively support L3 >>> tunnels, I think it's going to be necessary to make some incompatible >>> changes in the future. Given that, I would probably put the >>> identifier at the top of the non-upstream range (i.e. 105). >> >> When you say incompatible changes, do you mean breaking existing user >> scripts for setting up flows, etc.? I can only think of the MAC address >> rewrite that we have to do now as hack, since we don't do ARP >> resolution. That rule however would not break a future implementation, >> and is really simple to remove by users. Are there other things you >> foresee that would change? > > Well, currently OVS userspace will reject anything without an Ethernet > header. My assumption is that having native L3 support would involve > supporting this between userspace/kernel and then the LISP vport would > generate that type of packet. This change would break compatibility > since an old userspace would no longer be able to work with a new > kernel even though it is requesting the same type of port. Thanks for explaining, I see now. > > Mostly though, it just seems that there are too many pieces not yet > designed to be feel confident that it won't break compatibility. > Also, putting it in the non-upstream range doesn't preclude it from > being upstreamed - it just means that when we are ready we'll move it, > which will be a break in compatibility for mismatched userspace/kernel > versions (which we only support for features that are upstreamed). OK, good too know! ;) > >> If possible, we would like to see LISP as a candidate for upstreaming, >> and will try our best to avoid backwards compatible changes. >> >> BTW, how does upstreaming work? Is there a branch, which gets pulled by >> the netdev maintainers, when the kernel merge window opens? Or do you >> sned a pull request from latest master during the window? Or they pull >> point releases? How often do they pull changes? Every merge window? >> Or upstreaming is not tied to the kernel merge window, because it goes >> to the network subsystem tree first? I'm asking this to see if we can >> at least do the design of the L3 work before the next upstream code >> pull, and see how backward compatibility would be affected by the >> planned changes. > > There's an upstream OVS tree here: > http://git.kernel.org/?p=linux/kernel/git/jesse/openvswitch.git;a=summary > > It has to be maintained separately from the main OVS tree since the > paths are different and the openvswitch.org version also has things > like compatibility with different kernel versions that don't make > sense in the context of the mainline kernel. Usually I just > accumulate things going into master there and then send a pull request > once per cycle. > > Tunneling is a little more complicated though because none of the > infrastructure is there yet (Pravin is working on it right now). > Until that happens, I can't send anything that depends on it upstream > so it will just stay in the OVS repository. In any case though, I > don't send things upstream unless there is a pretty solid plan for how > to evolve them in a compatible way (which is why the tunneling stuff > is going so much later - the previous way of doing things just wasn't > extensible enough). So while I would also like to get this upstream, > there isn't any danger of it being upstreamed before it's ready. Thanks again for the detailed explanation. I will submit v3 once I know how to best handle the OVS_CB macro. -Lori _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev