netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: ggrundstrom@neteffect.com
Cc: netdev@vger.kernel.org, general@lists.openfabrics.org
Subject: Re: [ofa-general] [PATCH] RDMA/CMA: Implement rdma_resolve_ip retry enhancement.
Date: Wed, 19 Sep 2007 08:43:50 -0700	[thread overview]
Message-ID: <adabqbyk6dl.fsf@cisco.com> (raw)
In-Reply-To: <200709190022.l8J0MbWt024754@neteffect.com> (ggrundstrom@neteffect.com's message of "Tue, 18 Sep 2007 19:22:37 -0500")

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

  reply	other threads:[~2007-09-19 15:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-19  0:22 [ofa-general] [PATCH] RDMA/CMA: Implement rdma_resolve_ip retry enhancement ggrundstrom
2007-09-19 15:43 ` Roland Dreier [this message]
2007-09-19 16:52 ` Sean Hefty
2007-09-19 23:45   ` Glenn Grundstrom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adabqbyk6dl.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=general@lists.openfabrics.org \
    --cc=ggrundstrom@neteffect.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).