From: Pravin Shelar <pshelar@nicira.com>
To: Timo Teras <timo.teras@iki.fi>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net v2] ip_tunnels: Use skb-len to PMTU check.
Date: Mon, 1 Jul 2013 00:06:22 -0700 [thread overview]
Message-ID: <CALnjE+qW2P031bwOaTkC-kP2LOvOydBQ0YzBAhd71D66Wugfig@mail.gmail.com> (raw)
In-Reply-To: <20130701094132.3019afb4@vostro>
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.
> - Timo
next prev parent reply other threads:[~2013-07-01 7:06 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 [this message]
2013-07-01 12:18 ` Timo Teras
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=CALnjE+qW2P031bwOaTkC-kP2LOvOydBQ0YzBAhd71D66Wugfig@mail.gmail.com \
--to=pshelar@nicira.com \
--cc=netdev@vger.kernel.org \
--cc=timo.teras@iki.fi \
/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).