All of lore.kernel.org
 help / color / mirror / Atom feed
From: Neil Brown <neilb@suse.de>
To: Olaf Kirch <olaf.kirch@oracle.com>
Cc: netdev@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>,
	nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 001 of 3] knfsd: Use recv_msg to get peer address for NFSD instead of code-copying
Date: Tue, 6 Mar 2007 08:09:12 +1100	[thread overview]
Message-ID: <17900.34552.69250.247125@notabene.brown> (raw)
In-Reply-To: message from Olaf Kirch on Monday March 5

On Monday March 5, olaf.kirch@oracle.com wrote:
> 
> Hi Neil,
> 
> here's another minor comment:
> 
> On Friday 02 March 2007 05:28, NeilBrown wrote:
> > +static inline void svc_udp_get_dest_address(struct svc_rqst *rqstp,
> > +					    struct cmsghdr *cmh)
> >  {
> >  	switch (rqstp->rq_sock->sk_sk->sk_family) {
> >  	case AF_INET: {
> > +		struct in_pktinfo *pki = CMSG_DATA(cmh);
> > +		rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
> >  		break;
> > +		}
> ...
> 
> The daddr that is extracted here will only ever be used to build
> another PKTINFO cmsg when sending the reply. So it would be
> much easier to just store the raw control message in the svc_rqst,
> without looking at its contents, and send it out along with the reply,
> unchanged.

Yes, sounds tempting, doesn't it?
Unfortunately it isn't that simple as I found out when the sunrpc code
in glibc did exactly that.

You see sendmsg will use the interface-number as well as the source
address from the PKTINFO structure.

Suppose my server has two interfaces (A and B) on two subnets that
both are connected to some router which is connected to a third subnet
that my client is on.  Further, suppose my server has only one default
route, out interface A.
The client chooses the IP address of interface B and sends a request.
It arrives on interface B and is processed.
If the PKTINFO received is passed unchanged to sendmsg, the pack will
be sent out interface B.  But interfacve B doesn't have a route to
that client, so the packet is dropped.

This exactly what was happening for me with mountd a few years ago.

So yes, we could just zero the interface field, but I think it is
clearer to extract that wanted data, then re-insert it.  They really
are different structures with different meanings (send verse receive)
which happen to have the same layout.

Thanks,
NeilBrown


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

WARNING: multiple messages have this Message-ID (diff)
From: Neil Brown <neilb@suse.de>
To: Olaf Kirch <olaf.kirch@oracle.com>
Cc: nfs@lists.sourceforge.net,
	Andrew Morton <akpm@linux-foundation.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [NFS] [PATCH 001 of 3] knfsd: Use recv_msg to get peer address for NFSD instead of code-copying
Date: Tue, 6 Mar 2007 08:09:12 +1100	[thread overview]
Message-ID: <17900.34552.69250.247125@notabene.brown> (raw)
In-Reply-To: message from Olaf Kirch on Monday March 5

On Monday March 5, olaf.kirch@oracle.com wrote:
> 
> Hi Neil,
> 
> here's another minor comment:
> 
> On Friday 02 March 2007 05:28, NeilBrown wrote:
> > +static inline void svc_udp_get_dest_address(struct svc_rqst *rqstp,
> > +					    struct cmsghdr *cmh)
> >  {
> >  	switch (rqstp->rq_sock->sk_sk->sk_family) {
> >  	case AF_INET: {
> > +		struct in_pktinfo *pki = CMSG_DATA(cmh);
> > +		rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
> >  		break;
> > +		}
> ...
> 
> The daddr that is extracted here will only ever be used to build
> another PKTINFO cmsg when sending the reply. So it would be
> much easier to just store the raw control message in the svc_rqst,
> without looking at its contents, and send it out along with the reply,
> unchanged.

Yes, sounds tempting, doesn't it?
Unfortunately it isn't that simple as I found out when the sunrpc code
in glibc did exactly that.

You see sendmsg will use the interface-number as well as the source
address from the PKTINFO structure.

Suppose my server has two interfaces (A and B) on two subnets that
both are connected to some router which is connected to a third subnet
that my client is on.  Further, suppose my server has only one default
route, out interface A.
The client chooses the IP address of interface B and sends a request.
It arrives on interface B and is processed.
If the PKTINFO received is passed unchanged to sendmsg, the pack will
be sent out interface B.  But interfacve B doesn't have a route to
that client, so the packet is dropped.

This exactly what was happening for me with mountd a few years ago.

So yes, we could just zero the interface field, but I think it is
clearer to extract that wanted data, then re-insert it.  They really
are different structures with different meanings (send verse receive)
which happen to have the same layout.

Thanks,
NeilBrown


  reply	other threads:[~2007-03-05 21:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-02  4:28 [PATCH 000 of 3] knfsd: Resolve IPv6 related link error NeilBrown
2007-03-02  4:28 ` [PATCH 001 of 3] knfsd: Use recv_msg to get peer address for NFSD instead of code-copying NeilBrown
2007-03-02  4:28   ` NeilBrown
2007-03-05 18:53   ` Olaf Kirch
2007-03-05 18:53     ` [NFS] " Olaf Kirch
2007-03-05 23:47     ` Neil Brown
2007-03-05 18:59   ` Olaf Kirch
2007-03-05 21:09     ` Neil Brown [this message]
2007-03-05 21:09       ` Neil Brown
2007-03-02  4:28 ` [PATCH 002 of 3] knfsd: Avoid checksum checks when collecting metadata for a UDP packet NeilBrown
2007-03-02  4:28   ` NeilBrown
2007-03-02  4:28 ` [PATCH 003 of 3] knfsd: Remove CONFIG_IPV6 ifdefs from sunrpc server code NeilBrown
2007-03-02  4:28   ` NeilBrown

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=17900.34552.69250.247125@notabene.brown \
    --to=neilb@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nfs@lists.sourceforge.net \
    --cc=olaf.kirch@oracle.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.