linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: praveen chaudhary <praveen5582@gmail.com>
To: David Ahern <dsahern@gmail.com>, Jakub Kicinski <kuba@kernel.org>
Cc: "Maciej Żenczykowski" <zenczykowski@gmail.com>,
	"David Miller" <davem@davemloft.net>,
	corbet@lwn.net, "Alexey Kuznetsov" <kuznet@ms2.inr.ac.ru>,
	"Hideaki Yoshifuji" <yoshfuji@linux-ipv6.org>,
	"Linux NetDev" <netdev@vger.kernel.org>,
	linux-doc@vger.kernel.org,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Zhenggen Xu" <zxu@linkedin.com>
Subject: Re: [PATCH v3 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
Date: Sun, 24 Jan 2021 00:09:20 -0800	[thread overview]
Message-ID: <3ACC52AF-8992-4CD9-B89F-2529DA402A25@gmail.com> (raw)
In-Reply-To: <2de4989b-5b72-cb0a-470f-9bd0dcb96e53@gmail.com>



> On Jan 23, 2021, at 5:13 PM, David Ahern <dsahern@gmail.com> wrote:
> 
> On 1/23/21 1:00 PM, Jakub Kicinski wrote:
>> On Fri, 22 Jan 2021 22:16:41 -0700 David Ahern wrote:
>>> On 1/22/21 9:02 PM, Maciej Żenczykowski wrote:
>>>> Why can't we get rid of the special case for 0 and simply make 1024 the
>>>> default value?  
>>> 
>>> That would work too.
>> 
>> Should we drop it then? Easier to bring it back than to change the
>> interpretation later. It doesn't seem to serve any clear purpose right
>> now.
>> 
>> (Praveen if you post v4 please take a look at the checkpatch --strict
>> warnings and address the ones which make sense, e.g. drop the brackets
>> around comparisons, those are just noise, basic grasp of C operator
>> precedence can be assumed in readers of kernel code).
>> 
> 
> let's do a v4.
> 
> Praveen: set the initial value to IP6_RT_PRIO_USER, do not allow 0,
> remove the checks on value and don't forget to update documentation.
> 

Sure, I will respin V4, with above mentioned changes. Also, I will address checkpatch --strict warnings.

I wanted to set initial value to IP6_RT_PRIO_USER in v1, but avoided till review for 2 simple coding reasons:
1.) IP6_RT_PRIO_USER must be exposed in net/ipv6/addrconf.c by including include/uapi/linux/ipv6_route.h.
2.) If rt6_add_dflt_router() will be called from other files in future, IP6_RT_PRIO_USER should be included in all those files as well, because caller will pass most probably default value.

> Oh and cc me on the next otherwise the review depends on me finding time
> to scan netdev.

Sure, I will cc you and will add “Reviewed by” as well. I will also send you the lkml link to v4.
Thanks Jakub and you for reviewing this over the weekend.



      reply	other threads:[~2021-01-24  8:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19 21:29 [PATCH v3 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement Praveen Chaudhary
2021-01-22 15:55 ` David Ahern
     [not found]   ` <CAHo-Oozz-mGNz4sphOJekNeAgGJCLmiZaiNccXjiQ02fQbfthQ@mail.gmail.com>
2021-01-23  5:16     ` David Ahern
2021-01-23 20:00       ` Jakub Kicinski
2021-01-24  1:13         ` David Ahern
2021-01-24  8:09           ` praveen chaudhary [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=3ACC52AF-8992-4CD9-B89F-2529DA402A25@gmail.com \
    --to=praveen5582@gmail.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=yoshfuji@linux-ipv6.org \
    --cc=zenczykowski@gmail.com \
    --cc=zxu@linkedin.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).