All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gao Feng" <gfree.wind@foxmail.com>
To: "'Joe Perches'" <joe@perches.com>, <gfree.wind@foxmail.com>,
	<davem@davemloft.net>, <kuznet@ms2.inr.ac.ru>,
	<jmorris@namei.org>, <netdev@vger.kernel.org>
Subject: RE: [PATCH net-next 1/1] net: ipv4: Refine the ipv4_default_advmss
Date: Wed, 12 Apr 2017 10:33:22 +0800	[thread overview]
Message-ID: <000601d2b335$28431810$78c94830$@foxmail.com> (raw)
In-Reply-To: <1491962594.17839.28.camel@perches.com>

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Wednesday, April 12, 2017 10:03 AM
> To: gfree.wind@foxmail.com; davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> jmorris@namei.org; netdev@vger.kernel.org
> Cc: Gao Feng <fgao@ikuai8.com>
> Subject: Re: [PATCH net-next 1/1] net: ipv4: Refine the
ipv4_default_advmss
> 
> On Wed, 2017-04-12 at 09:36 +0800, gfree.wind@foxmail.com wrote:
> > From: Gao Feng <fgao@ikuai8.com>
> >
> > 1. Don't get the metric RTAX_ADVMSS of dst.
> > There are two reasons.
> > 1) Its caller dst_metric_advmss has already invoke dst_metric_advmss
> > before invoke default_advmss.
> > 2) The ipv4_default_advmss is used to get the default mss, it should
> > not try to get the metric like ip6_default_advmss.
> >
> > 2. Use sizeof(tcphdr)+sizeof(iphdr) instead of literal 40.
> >
> > 3. Define one new macro IPV4_MAX_PMTU instead of 65535 according to
> > RFC 2675, section 5.1.
> 
> trivia:
> 
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> []
> > @@ -1250,14 +1250,11 @@ static void set_class_tag(struct rtable *rt,
> > u32 tag)
> >
> >  static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
> > {
> > -	unsigned int advmss = dst_metric_raw(dst, RTAX_ADVMSS);
> > +	unsigned int header_size = sizeof(struct tcphdr) + sizeof(struct
iphdr);
> > +	unsigned int advmss = max_t(unsigned int, dst->dev->mtu -
header_size,
> > +				    ip_rt_min_advmss);
> >
> > -	if (advmss == 0) {
> > -		advmss = max_t(unsigned int, dst->dev->mtu - 40,
> > -			       ip_rt_min_advmss);
> > -		if (advmss > 65535 - 40)
> > -			advmss = 65535 - 40;
> > -	}
> > +	advmss = min_t(unsigned int, advmss, IPV4_MAX_PMTU - header_size);
> 
> as all the elements are now unsigned int, the min_t cast seems
unnecessary.

Thanks, it is unnecessary to use min_t.
I have corrected it now.

Best Regards
Feng

      reply	other threads:[~2017-04-12  2:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-12  1:36 [PATCH net-next 1/1] net: ipv4: Refine the ipv4_default_advmss gfree.wind
2017-04-12  2:03 ` Joe Perches
2017-04-12  2:33   ` Gao Feng [this message]

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='000601d2b335$28431810$78c94830$@foxmail.com' \
    --to=gfree.wind@foxmail.com \
    --cc=davem@davemloft.net \
    --cc=jmorris@namei.org \
    --cc=joe@perches.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=netdev@vger.kernel.org \
    /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.