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

[Andy Wang]

Hi Roopa,

I read the difference which you list below carefully. It is good to me and should be able to fix the problem I met. 
Since you have prepared the change , I think I needn't to submit the patch again. What I am expecting is that the change is summited to the new version of Libnl ASAP since it is blocking QA's testing.
Let's wait for the suggestions from Thomas and his final approval. Thanks for your update.

-YuTong Wang


-----Original Message-----
From: Roopa Prabhu [mailto:roopa at cumulusnetworks.com] 
Sent: Monday, January 28, 2013 4:14 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

I had added the same check in rtnl_link_get. Maybe that can be reversed too.

In all, the below patch can be reversed.

thanks.

diff --git a/lib/route/link.c b/lib/route/link.c index f8646d1..b378f30 100644
--- a/lib/route/link.c
+++ b/lib/route/link.c
@@ -966,7 +955,7 @@ struct rtnl_link *rtnl_link_get(struct nl_cache *cache, int ifindex)
  		return NULL;

  	nl_list_for_each_entry(link, &cache->c_items, ce_list) {
-		if (link->l_index == ifindex) {
+		if (link->l_family == AF_UNSPEC && link->l_index == ifindex) {
  			nl_object_get((struct nl_object *) link);
  			return link;
  		}
@@ -999,7 +988,8 @@ struct rtnl_link *rtnl_link_get_by_name(struct nl_cache *cache,
  		return NULL;

  	nl_list_for_each_entry(link, &cache->c_items, ce_list) {
-		if (!strcmp(name, link->l_name)) {
+		if (link->l_family == AF_UNSPEC &&
+			!strcmp(name, link->l_name)) {
  			nl_object_get((struct nl_object *) link);
  			return link;
  		}


On 1/27/13 11:12 PM, Roopa Prabhu wrote:
> 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.
>
> _______________________________________________
> libnl mailing list
> libnl at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libnl