Ask for help about the change of libnl 3.2.16 or newer version

[Xiaodong Xu]

Andy,

I think you can make changes by yourself since we have reviewers, right?
Count Rich in when your patch is ready.
We can contribute to the open source community and that is how we benefit from it.

Regards,
Shelton (Xiaodong)

On Jan 28, 2013, at 16:54, "Andy Wang" <Andy.Wang at watchguard.com> wrote:

> Hi Roopa,
> 
> Thanks for your quick replay. I think it should be better if you submit a patch instead of me ,since you know more in this field than me. Please let me know if the patch is prepared.
> 
> -YuTong Wang
> 
> -----Original Message-----
> From: Roopa Prabhu [mailto:roopa at cumulusnetworks.com] 
> Sent: Monday, January 28, 2013 3:13 PM
> To: libnl at lists.infradead.org; Andy Wang
> Subject: Re: Ask for help about the change of libnl 3.2.16 or newer version
> 
> On 1/27/13 9:28 PM, Roopa Prabhu wrote:
>> On 1/27/13 8:50 AM, Andy Wang wrote:
>>> Hi friends of Libnl organization:
>>> 
>>> I am a Watchguard employee and quite new to Libnl framework. In about 
>>> Jan 5^th we upgraded to the new version 3.2.16 . The new version is 
>>> good ,but we found we cannot add the IPV6 address in the new version.
>>> 
>>> Function "rtnl_link_name2i" in this version always return integer 
>>> value *0*which means it fails to get a valid interface index from 
>>> cache. I also tried the newest version 3.2.20, it has the same problem.
>>> 
>>> I copied the function of our company in which it called Libnl 
>>> function to add IPV6 address. It works well in the past version 
>>> before 3.2.16, but it failed in the line which is drawn as green after it.
>>> 
>>> -------------------------
>>> 
>>> /* This adds single address to interface */
>>> 
>>> int wgnet_add_addr(const char * ifname,struct if_addr * ip) {
>>> 
>>> struct nl_cache *link_cache = NULL;
>>> 
>>> struct nl_addr *n_addr = NULL;
>>> 
>>> struct rtnl_addr *r_addr = NULL;
>>> 
>>> int ifindex;
>>> 
>>> int ret=-1;
>>> 
>>> if (!ip || !ifname) {
>>> 
>>> return EINVAL;
>>> 
>>> }
>>> 
>>> if (!handle && (ret=wgnet_init()) != 0 ) {
>>> 
>>> return ret;
>>> 
>>> }
>>> 
>>> rtnl_link_alloc_cache(handle,ip->ipaddr.sa_family,&link_cache);
>>> 
>>> r_addr = rtnl_addr_alloc();
>>> 
>>> /* FIXME we can use local version at some point */
>>> 
>>> n_addr=nl_addr_alloc(sizeof(ip->ipaddr.ip6));
>>> 
>>> if (!link_cache || !r_addr || !n_addr) {
>>> 
>>> ret = ENOMEM;
>>> 
>>> goto error;
>>> 
>>> }
>>> 
>>> **
>>> 
>>> *ifindex = rtnl_link_name2i(link_cache, ifname); *
>>> 
>>> if (!ifindex) {
>>> 
>>> ret = ENOENT;
>>> 
>>> goto error;
>>> 
>>> }
>>> 
>>> /* Copy data from provided data */
>>> 
>>> nl_addr_set_family(n_addr,ip->ipaddr.sa_family);
>>> 
>>> nl_addr_set_binary_addr(n_addr,ip->ipaddr.addr,ip->ipaddr.sa_family==
>>> AF_INET
>>> 
>>> ? sizeof(ip->ipaddr.ip): sizeof(ip->ipaddr.ip6)) ;
>>> 
>>> nl_addr_set_prefixlen(n_addr,ip->ipaddr.prefix);
>>> 
>>> /* set netlink message and sent it out */
>>> 
>>> rtnl_addr_set_local(r_addr,n_addr);
>>> 
>>> rtnl_addr_set_ifindex(r_addr, ifindex);
>>> 
>>> if ( ip->scope ) {
>>> 
>>> rtnl_addr_set_scope(r_addr,ip->scope);
>>> 
>>> }
>>> 
>>> if ( ip->flags ) {
>>> 
>>> rtnl_addr_set_flags(r_addr,ip->flags);
>>> 
>>> }
>>> 
>>> ret=rtnl_addr_add(handle, r_addr, 0);
>>> 
>>> error:
>>> 
>>> *****
>>> 
>>> return ret;
>>> 
>>> }
>>> 
>>> -------------------------
>>> 
>>> I compared the source code of the past version 3.2.13 and the current 
>>> version 3.2.16 , and found there are the following modification in 
>>> function "rtnl_link_get_by_name" which "rtnl_link_name2i" calls.
>>> 
>>> Then I comment this new-added code and build the newlibnl* .so files, 
>>> and use them again. Then the IPV6 address could be added rightly like 
>>> before.
>>> 
>>> I think there should have some reasons why the new code is added, but 
>>> I can't understand more.
>> 
>> This was something I added. Sorry if this broke your code.
>> This code was added during adding AF_BRIDGE support to link cache.
>> 
>> The reason this was added is, link cache can now contain objects of 
>> AF_BRIDGE, And i wanted to make sure rtnl_link_get_by_name did not 
>> break and return AF_BRIDGE objects.
>> 
>> But from what you are saying, rtnl_link_get_by_name returned the first 
>> link object with matching ifindex and that could have also been an 
>> object of AF_INET6 family. And i assumed that this was only of family 
>> AF_UNSPEC.
>> 
>> A straightforward fix is to also add the AF_INET6 check in 
>> rtnl_link_get_by_name there.
>> 
>> Or maybe there is a better solution. Let me think about this some more.
>> 
> 
> I am leaning towards removing the AF_UNSPEC check (ie undoing the update to rtnl_link_get_by_name). That way this api will return the first object. It could be of any family. The user will know if the cache contains bridge objects, because they are available only with the cache flag NL_CACHE_AF_ITER.
> 
> If the user wants to search for a specific object, he can always use nl_cache_find.
> 
> Thomas, does this sound ok. ?
> 
> Andy, If thomas approves, since you found the problem, will you be submitting a patch ?. If not, let me know, I can. Thanks.