[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220405174743.153b1a36@kernel.org>
Date: Tue, 5 Apr 2022 17:47:43 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Florent Fourcot <florent.fourcot@...irst.fr>
Cc: netdev@...r.kernel.org, cong.wang@...edance.com,
edumazet@...gle.com, Brian Baboch <brian.baboch@...irst.fr>
Subject: Re: [PATCH v3 net-next 3/4] rtnetlink: return ENODEV when ifname
does not exist and group is given
On Tue, 5 Apr 2022 15:42:36 +0200 Florent Fourcot wrote:
> Changes in v3:
> * Use a boolean to have condition duplication
>
> Signed-off-by: Florent Fourcot <florent.fourcot@...irst.fr>
> Signed-off-by: Brian Baboch <brian.baboch@...irst.fr>
> ---
> net/core/rtnetlink.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 33dbeed7e531..e93f4058cf08 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3279,6 +3279,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct net *dest_net, *link_net;
> struct nlattr **slave_data;
> char kind[MODULE_NAME_LEN];
> + bool link_lookup = false;
link_specified ? Somehow link_lookup does not speak to me.
> struct net_device *dev;
> struct ifinfomsg *ifm;
> char ifname[IFNAMSIZ];
> @@ -3298,12 +3299,15 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> return err;
>
> ifm = nlmsg_data(nlh);
> - if (ifm->ifi_index > 0)
> + if (ifm->ifi_index > 0) {
> + link_lookup = true;
> dev = __dev_get_by_index(net, ifm->ifi_index);
> - else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
> + } else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) {
> + link_lookup = true;
> dev = rtnl_dev_get(net, tb);
> - else
> + } else {
> dev = NULL;
maybe set it to false here and don't do the initialization.
It doesn't matter in practice but the function has a "retry"
goto which skips the init.
> + }
>
> master_dev = NULL;
> m_ops = NULL;
> @@ -3405,8 +3409,14 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> return do_setlink(skb, dev, ifm, extack, tb, status);
> }
>
> +
spurious new line
> if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
> - if (ifm->ifi_index == 0 && tb[IFLA_GROUP])
> + /* No dev found and NLM_F_CREATE not set. Requested dev does not exist,
> + * or it's for a group
> + */
> + if (link_lookup)
> + return -ENODEV;
> + if (tb[IFLA_GROUP])
Powered by blists - more mailing lists