[systemd-devel] [PATCH] networkd: Introduce ipip tunnel

[Tom Gundersen]

On Mon, Apr 7, 2014 at 6:35 AM, Susant Sahani <susant at redhat.com> wrote:
> I have addressed all your comments.

Cool.

> However I have some queries
> Please find below.
>> Hm, we can probably reuse some of the existing address parsing
>> functions don't you think? And we should also check the address
>> faimilies here right?
>
>
> I think I can reuse net_parse_inaddr .

Yes, sounds like the right thing to do. It will also allow you to
force the address family and fail if it is wrong.

>> Hm, I guess these should be _append_in_addr() to get the typesafety
>> right (might need to verify that we are using the right types for this
>> in rtnl-types.c.
>
>  I am missing something in the code . with the current rtnl code
> it does not get appended.  Could you please give a example.

Your follow-up patch does the right thing. The types were sort of
wrong in the library, but it would not be noticed by the kernel as
NLA_IN_ADDR is equivalent to NLA_U32 for IPv4 addresses. However, it
is not for IPv6 addresses, so better to do this explicitly as your
next patch suggests.

>> This should be fixed in the kernel I think. All that is needed is to
>> add MODULE_ALIA_RTNL_LINK("ipip") to the ipip module (and the same for
>> the other modules). This is already done for many (most?) netdev
>> kinds, which is why this stuff "just works" for bridges, bonds, etc.
>
> I am not sure how it will be fixed in  kernel. If the module not present
> kernel will say not supported . Could you please give a example.
> The main reason for loading module from networkd is avoid users
> loading manually .

The kernel will auto-load any missing module as long as it has the
correct module alias. If it finds that a certain "kind" is unsupported
it will try to load "rtnl-link-<kind>". Have a look at "modinfo
bridge" and compare with "modinfo ipip" to see why the one works and
the other does not. I'll submit a kernel patch for this (but until we
know whether or not that will be applied I'd just keep the module
loading you currently have).

Thanks!

Tom