From: Timo Teras <timo.teras@iki.fi>
To: Pravin Shelar <pshelar@nicira.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net v2] ip_tunnels: Use skb-len to PMTU check.
Date: Mon, 1 Jul 2013 15:18:39 +0300 [thread overview]
Message-ID: <20130701151839.1e7dd12d@vostro> (raw)
In-Reply-To: <CALnjE+qW2P031bwOaTkC-kP2LOvOydBQ0YzBAhd71D66Wugfig@mail.gmail.com>
On Mon, 1 Jul 2013 00:06:22 -0700
Pravin Shelar <pshelar@nicira.com> wrote:
> On Sun, Jun 30, 2013 at 11:41 PM, Timo Teras <timo.teras@iki.fi>
> wrote:
> > On Sun, 30 Jun 2013 23:30:32 -0700
> > Pravin B Shelar <pshelar@nicira.com> wrote:
> >
> >> In path mtu check, ip header total length works for gre device
> >> but not for gre-tap device. Use skb len which is consistent
> >> for all tunneling types. This is old bug in gre.
> >> This also fixes mtu calculation bug introduced by
> >> commit c54419321455631079c7d (GRE: Refactor GRE tunneling code).
> >>
> >> Reported-by: Timo Teras <timo.teras@iki.fi>
> >> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> >> ---
> >> v1-v2:
> >> - Fix pmtu set.
> >> - This patch also restructures code which help couple of
> >> improvements I have.
> >
> > Looks good to me. One additional comment below.
> >
> >> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> >> index 7fa8f08..ae5b78c 100644
> >> --- a/net/ipv4/ip_tunnel.c
> >> +++ b/net/ipv4/ip_tunnel.c
> >> @@ -486,6 +486,53 @@ drop:
> >> }
> >> EXPORT_SYMBOL_GPL(ip_tunnel_rcv);
> >>
> >> +static int tnl_update_pmtu(struct net_device *dev, struct sk_buff
> >> *skb,
> >> + struct rtable *rt, __be16 df)
> >> +{
> >> + struct ip_tunnel *tunnel = netdev_priv(dev);
> >> + int pkt_size = skb->len - tunnel->hlen;
> >> + int mtu;
> >> +
> >> + if (df)
> >> + mtu = dst_mtu(&rt->dst) - dev->hard_header_len
> >> + - sizeof(struct iphdr) -
> >> tunnel->hlen;
> >> + else
> >> + mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) :
> >> dev->mtu; +
> >> + if (skb_dst(skb))
> >> + skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL,
> >> skb, mtu); +
> >
> > Since mtu can change for skb_dst() only if df is set, would it make
> > sense to move the whole update_pmtu call inside if (df) {} block?
> >
> I am not sure abt that. Other events can change mtu.
> anyways I think it would be better to have separate patch for that if
> required. This patch already fixes two (related) issues.
Fair enough.
Acked-by: Timo Teräs <timo.teras@iki.fi>
next prev parent reply other threads:[~2013-07-01 12:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-01 6:30 [PATCH net v2] ip_tunnels: Use skb-len to PMTU check Pravin B Shelar
2013-07-01 6:41 ` Timo Teras
2013-07-01 7:06 ` Pravin Shelar
2013-07-01 12:18 ` Timo Teras [this message]
2013-07-02 6:56 ` David 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=20130701151839.1e7dd12d@vostro \
--to=timo.teras@iki.fi \
--cc=netdev@vger.kernel.org \
--cc=pshelar@nicira.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).