netdev - Re: [ofa-general] [PATCH] RDMA/CMA: Implement rdma_resolve

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <adabqbyk6dl.fsf@cisco.com>
Date:	Wed, 19 Sep 2007 08:43:50 -0700
From:	Roland Dreier <rdreier@...co.com>
To:	ggrundstrom@...effect.com
Cc:	netdev@...r.kernel.org, general@...ts.openfabrics.org
Subject: Re: [ofa-general] [PATCH] RDMA/CMA: Implement rdma_resolve_ip retry enhancement.

Thanks for the patch...

 > If an application is calling rdma_resolve_ip() and a status of -ENODATA is returned from addr_resolve_local/remote(), the timeout mechanism waits until the application's timeout occurs before rechecking the address resolution status; the application will wait until it's full timeout occurs.  This case is seen when the work thread call to process_req() is made before the arp packet is processed.

I'm having a hard time understanding this changelog.  Could you please
resend with a description that lets me understand:

 - What the current behavior is and what is wrong with that;
 - What the behavior should be;
 - And how your patch changes the behavior to be correct.

 > This patch is in addition to Steve Wise's neigh_event_send patch to initiate neighbour discovery sent on 9/12/2007.

Does this mean it depends on Steve's patch being applied first?

Also please try to keep lines in the changelog to 72 characters or so...

 > @@ -136,6 +137,7 @@ static void set_timeout(unsigned long ti
 >  static void queue_req(struct addr_req *req)
 >  {
 >  	struct addr_req *temp_req;
 > +	unsigned long req_timeout = msecs_to_jiffies(MIN_ADDR_TIMEOUT_MS) + jiffies;
 >  
 >  	mutex_lock(&lock);
 >  	list_for_each_entry_reverse(temp_req, &req_list, list) {
 > @@ -145,8 +147,10 @@ static void queue_req(struct addr_req *r
 >  
 >  	list_add(&req->list, &temp_req->list);
 >  
 > -	if (req_list.next == &req->list)
 > +	if (req_list.next == &req->list) {
 > +		req_timeout = min(req_timeout, req->timeout);
 >  		set_timeout(req->timeout);
 > +	}
 >  	mutex_unlock(&lock);
 >  }

I don't understand this code.  It seems you keep track of the minimum
timeout, and then ignore the value you computed.  What am I missing?

Thanks,
  Roland
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html