[ovs-dev] [PATCHv3.1] Add support for LISP tunneling

Lori Jakab lojakab at cisco.com
Thu Feb 21 09:42:27 UTC 2013


On 02/21/13 00:50, Jesse Gross wrote:
> On Wed, Feb 20, 2013 at 2:33 AM, Lorand Jakab <lojakab at cisco.com> wrote:
>> LISP is an experimental layer 3 tunneling protocol, described in RFC
>> 6830.  This patch adds support for LISP tunneling.  Since LISP
>> encapsulated packets do not carry an Ethernet header, it is removed
>> before encapsulation, and added with hardcoded source and destination
>> MAC addresses after decapsulation.  The harcoded MAC chosen for this
>> purpose is the locally administered address 02:00:00:00:00:00.  Flow
>> actions can be used to rewrite this MAC for correct reception.  As such,
>> this patch is intended to be used for static network configurations, or
>> with a LISP capable controller.
>>
>> Signed-off-by: Lorand Jakab <lojakab at cisco.com>
>> Signed-off-by: Kyle Mestery <kmestery at cisco.com>
> 
> I'm still getting some sparse warnings, can you take a look?:
> 
>   CHECK   /home/jgross/openvswitch/datapath/linux/vport-lisp.c
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:177:18: warning:
> restricted __be64 degrades to integer
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:177:51: warning:
> restricted __be64 degrades to integer
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:178:18: warning:
> restricted __be64 degrades to integer
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:177:80: warning:
> incorrect type in return expression (different base types)
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:177:80:
> expected restricted __be64
> /home/jgross/openvswitch/datapath/linux/vport-lisp.c:177:80:    got
> unsigned long long

I tried reproducing this using sparse 0.4.3, 0.4.4, and latest git, on
Gentoo and Fedora (but I don't think distribution matters), against a
3.7 and a 3.8 kernel, with `make C=1`.  Neither combination generated
the above warnings for me.  Can you please describe your setup, so that
I can try reproducing and then fixing these warnings?

> 
>> diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
>> new file mode 100644
>> index 0000000..14060f1
>> --- /dev/null
>> +++ b/datapath/vport-lisp.c
>> +/* Called with rcu_read_lock and BH disabled. */
>> +static int lisp_rcv(struct sock *sk, struct sk_buff *skb)
> [...]
>> +       /* Add Ethernet header */
>> +       skb_push(skb, ETH_HLEN);
>> +
>> +       ethh = (struct ethhdr *)skb->data;
> 
> Did you mention that you were going to combine these lines?

Sorry about that, will fix in the next revision.

> 
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 88817ac..ddb9094 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> +static bool
>> +netdev_vport_is_lisp(const struct netdev *netdev)
>> +{
>> +    const struct netdev_dev *dev = netdev_get_dev(netdev);
>> +    const struct netdev_class *class = netdev_dev_get_class(dev);
>> +    const char *type = netdev_dev_get_type(dev);
>> +
>> +    return (class->get_config == get_tunnel_config
>> +            && !strcmp("lisp", type));
>> +}
>> +
>> +static bool
>> +netdev_vport_needs_port_number(const struct netdev *netdev)
>> +{
>> +    return (netdev_vport_is_vxlan(netdev) ||
>> +            netdev_vport_is_lisp(netdev));
>> +}
> 
> It might make sense to combine these into a single function.
> Hopefully, we don't need a lot of special casing for different types
> otherwise.

I wasn't sure which one you would prefer, so I went with the more
generalized (and readable?) solution.  Will change, no problem.

> 
>> @@ -318,7 +339,7 @@ set_tunnel_config(struct netdev_dev *dev_, const struct smap *args)
>>      ipsec_mech_set = false;
>>      memset(&tnl_cfg, 0, sizeof tnl_cfg);
>>
>> -    needs_dst_port = !strcmp(type, "vxlan");
>> +    needs_dst_port = !strcmp(type, "vxlan") || !strcmp(type, "lisp");
> 
> Can we use the new netdev_vport_needs_port_number() here?

Sure, but then I will rename netdev_vport_needs_port_number() to
netdev_vport_needs_dst_port() and change the argument type to 'const
struct netdev_dev' instead of 'const struct netdev'.

Let me know how to proceed with the sparse warnings, and I will send an
updated patch.

Thanks for reviewing,
-Lori



More information about the dev mailing list