On Mon, Apr 7, 2014 at 6:35 AM, Susant Sahani <sus...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel