From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>,
davem@davemloft.net, netdev@vger.kernel.org, kernel-team@fb.com,
tariqt@mellanox.com, yishaih@mellanox.com,
linux-rdma@vger.kernel.org
Subject: Re: [PATCH net-next] mlx4: make sure to always set the port type
Date: Mon, 7 Sep 2020 20:51:55 +0300 [thread overview]
Message-ID: <20200907175155.GD421756@unreal> (raw)
In-Reply-To: <20200907093614.12231d6b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Mon, Sep 07, 2020 at 09:36:14AM -0700, Jakub Kicinski wrote:
> On Mon, 7 Sep 2020 09:48:30 +0300 Leon Romanovsky wrote:
> >>>> And can we call to devlink_port_type_*_set() without IS_ENABLED() check?
> >>>
> >>> It'll generate two netlink notifications - not the end of the world but
> >>> also doesn't feel super clean.
> >
> > I would say that such a situation is corner case during the driver init and
> > not an end of the world to see double netlink message.
>
> Could you spell out your reasoning here? Are you concerned about
> out-of-tree drivers?
Nothing fancy, I just didn't see users who compiled mlx4_core and used
it without eth/ib.
The corner case is because this double netlink can be seen only during
driver reload and only if port type wasn't set.
>
> I don't see how adding IS_ENABLED() to the condition outweighs
> the benefit of not having duplicated netlink notifications.
Readability?
Anyway, it doesn't matter.
Thanks
prev parent reply other threads:[~2020-09-07 17:52 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 20:06 [PATCH net-next] mlx4: make sure to always set the port type Jakub Kicinski
2020-09-06 7:27 ` Leon Romanovsky
2020-09-06 16:33 ` Jakub Kicinski
2020-09-07 6:21 ` Jiri Pirko
2020-09-07 6:48 ` Leon Romanovsky
2020-09-07 7:19 ` Jiri Pirko
2020-09-07 9:10 ` Leon Romanovsky
2020-09-07 16:34 ` Jakub Kicinski
2020-09-07 16:55 ` Jiri Pirko
2020-09-07 16:36 ` Jakub Kicinski
2020-09-07 17:51 ` Leon Romanovsky [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=20200907175155.GD421756@unreal \
--to=leon@kernel.org \
--cc=davem@davemloft.net \
--cc=jiri@resnulli.us \
--cc=kernel-team@fb.com \
--cc=kuba@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=tariqt@mellanox.com \
--cc=yishaih@mellanox.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 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.