All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Mi Jinlong <mijinlong@cn.fujitsu.com>,
	NFS <linux-nfs@vger.kernel.org>,
	Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Re: [PATCH] sunrpc: Add scope id to svc_addr_u for IPv6 LINKLOCAL address
Date: Mon, 22 Aug 2011 15:44:59 -0400	[thread overview]
Message-ID: <20110822194459.GC16261@fieldses.org> (raw)
In-Reply-To: <3CA77E10-4099-4DDB-BE7A-88395CC34C93@oracle.com>

On Mon, Aug 22, 2011 at 03:32:13PM -0400, Chuck Lever wrote:
> 
> On Aug 22, 2011, at 3:26 PM, J. Bruce Fields wrote:
> 
> > On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote:
> >> +/* 
> >> + * Add scope id for LINKLOCAL address
> >> + */
> >> +struct in6_addr_scopeid{
> >> +	struct in6_addr	sin6_addr;
> >> +	__u32		sin6_scope_id;
> >> +};
> >> +
> >> union svc_addr_u {
> >> -    struct in_addr	addr;
> >> -    struct in6_addr	addr6;
> >> +	struct in_addr		addr;
> >> +	struct in6_addr_scopeid	addr6;
> > 
> > By the way, is there any reason why nfsd really needs its own address
> > structure?  Shouldn't we use sockaddr_storage or something?  I feel like
> > we've got a little too much one-off address handling in nfsd.
> 
> That would be my only complaint about the patch.
> 
> I think we chose a smaller struct here to save space, and we could do that because we didn't need a port number or scope ID.  If a scope ID is indeed required, then we should consider something larger like a struct sockaddr_storage, IMO.

If we care about the size of struct svc_rqst: rq_vec and rq_pages fields
are 4k and 2k, respectively, and the rest is lost in the noise.

Mi Jinlong, would you have the time to try this?

	- Replace svc_addr_u by sockaddr_storage
	- See of that allows us to delete some lines of code in nfsd by
	  using standard helper functions instead of copying things by
	  hand.

You'd do that in one patch, then apply your scope-id fix on top of it as
a second patch.

--b.

  reply	other threads:[~2011-08-22 19:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-20 10:24 [PATCH] sunrpc: Add scope id to svc_addr_u for IPv6 LINKLOCAL address Mi Jinlong
2011-08-22 19:23 ` J. Bruce Fields
2011-08-22 19:39   ` Jeff Layton
2011-08-22 19:44     ` Chuck Lever
2011-08-22 21:08       ` J. Bruce Fields
2011-08-23 14:43       ` Jeff Layton
2011-08-24  8:00         ` Mi Jinlong
2011-08-24 15:15           ` Chuck Lever
2011-08-22 19:26 ` J. Bruce Fields
2011-08-22 19:32   ` Chuck Lever
2011-08-22 19:44     ` J. Bruce Fields [this message]
2011-08-23  9:40       ` Mi Jinlong
2011-08-24  5:58       ` Mi Jinlong
2011-08-24 18:39         ` J. Bruce Fields
2011-08-25  0:02           ` Mi Jinlong
2011-08-25  9:58         ` Jeff Layton
2011-08-25 14:30         ` Chuck Lever
2011-08-25 14:37           ` Chuck Lever
2011-08-26  6:58             ` Mi Jinlong
2011-08-26 22:43               ` J. Bruce Fields
2011-08-27 22:03                 ` Chuck Lever
2011-08-29 15:26               ` Jeff Layton
2011-08-29 15:30                 ` Chuck Lever
2011-08-29 17:09               ` Steve Dickson

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=20110822194459.GC16261@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=Trond.Myklebust@netapp.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mijinlong@cn.fujitsu.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.