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

[Roopa Prabhu]

Thanks Andy. I will resubmit the patch with proper comments in a short 
while.

On 1/28/13 3:04 AM, Andy Wang wrote:
> 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
>