netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "David S. Miller" <davem@redhat.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: netdev@oss.sgi.com
Subject: Re: [ROUTE] PMTU only works on half the time
Date: Mon, 1 Dec 2003 14:21:31 -0800	[thread overview]
Message-ID: <20031201142131.5da50a07.davem@redhat.com> (raw)
In-Reply-To: <20031201220509.GA20827@gondor.apana.org.au>

On Tue, 2 Dec 2003 09:05:09 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Mon, Dec 01, 2003 at 01:51:54PM -0800, David S. Miller wrote:
> > 
> > Herbert, do you see how the outer loop and the skey[] thing works in
> > this PMTU handling code?  This takes care of comparing both iph->saddr
> > and '0' against rt->rt_src.
> 
> It only takes care of fl4_src, not rt_src.

Indeed.

At the surface it looks like a bug, but look at the redirect handling
tests in ip_rt_redirect().  It's a very similar key comparison as
the PMTU code, just structured differently:

            if (rth->fl.fl4_dst != daddr ||
                rth->fl.fl4_src != skeys[i] ||
                rth->fl.fl4_tos != tos ||
                rth->fl.oif != ikeys[k] ||
                rth->fl.iif != 0) {
                    rthp = &rth->u.rt_next;
                    continue;
            }

            if (rth->rt_dst != daddr ||
                rth->rt_src != saddr ||
                rth->u.dst.error ||
                rth->rt_gateway != old_gw ||
                rth->u.dst.dev != dev)
                    break;

See?  He's not comparing rt->rt_src against skeys[] and therefore '0'
here either.

I think the tests might be like this for a reason.

I could see Alexey constructing this test wrong in one instance, but
in two instances where the tests were structured totally different in
each case is hard to believe.

Let me think about this some more, maybe you're right and the
error exists in both of these places.

  reply	other threads:[~2003-12-01 22:21 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-12-01 20:16 [ROUTE] PMTU only works on half the time Herbert Xu
2003-12-01 20:47 ` Herbert Xu
2003-12-01 21:51   ` David S. Miller
2003-12-01 22:05     ` Herbert Xu
2003-12-01 22:21       ` David S. Miller [this message]
2003-12-01 23:22         ` David S. Miller
2003-12-02 10:10           ` Herbert Xu
2003-12-02 10:27             ` David S. Miller
2003-12-02 10:33               ` Herbert Xu
2003-12-01 23:30   ` Julian Anastasov
2003-12-01 23:50     ` David S. Miller
2003-12-02  0:04       ` Julian Anastasov
2003-12-02  0:07       ` Julian Anastasov
2003-12-02  0:08         ` David S. Miller
2003-12-02  1:53       ` Julian Anastasov
2003-12-02 23:40       ` Julian Anastasov
2003-12-03  0:07         ` David S. Miller
2003-12-03  0:39         ` David S. Miller
2003-12-03  1:43           ` Julian Anastasov
2003-12-03 22:03           ` Julian Anastasov
2003-12-05 20:43             ` David S. Miller
2003-12-06  7:52               ` Julian Anastasov
2003-12-10 23:15                 ` David S. Miller

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=20031201142131.5da50a07.davem@redhat.com \
    --to=davem@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@oss.sgi.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 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).