[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