Message-ID: <20241224103224.GF171473@unreal>
Date: Tue, 24 Dec 2024 12:32:24 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Junxian Huang <huangjunxian6@...ilicon.com>
Cc: jgg@...pe.ca, selvin.xavier@...adcom.com, chengyou@...ux.alibaba.com,
	kaishen@...ux.alibaba.com, mustafa.ismail@...el.com,
	tatyana.e.nikolova@...el.com, yishaih@...dia.com, benve@...co.com,
	neescoba@...co.com, bryan-bt.tan@...adcom.com,
	vishnu.dasa@...adcom.com, zyjzyj2000@...il.com, bmt@...ich.ibm.com,
	linux-rdma@...r.kernel.org, linuxarm@...wei.com,
	linux-kernel@...r.kernel.org, tangchengchang@...wei.com,
	liyuyu6@...wei.com, linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC 00/12] RDMA: Support link status events dispatching
 in ib_core

On Fri, Nov 22, 2024 at 06:52:56PM +0800, Junxian Huang wrote:
> This series is to integrate a common link status event handler in
> ib_core as this functionality is needed by most drivers and
> implemented in very similar patterns. This is not a new issue but
> a restart of the previous work of our colleagues from several years
> ago, please see [1] and [2].
> 
> [1]: https://lore.kernel.org/linux-rdma/1570184954-21384-1-git-send-email-liweihang@hisilicon.com/
> [2]: https://lore.kernel.org/linux-rdma/20200204082408.18728-1-liweihang@huawei.com/
> 
> With this series, ib_core can handle netdev events of link status,
> i.e. NETDEV_UP, NETDEV_DOWN and NETDEV_CHANGE, and dispatch ib port
> events to ULPs instead of drivers. However some drivers currently
> have some private processing in their handler, rather than simply
> dispatching events. For these drivers, this series provides a new
> ops report_port_event(). If this ops is set, ib_core will call it
> and the events will still be handled in the driver.
> 
> Events of LAG devices are also not handled in ib_core as currently
> there is no way to obtain ibdev from upper netdev in ib_core. This
> can be a TODO work after the core have more support for LAG. For
> now mlx5 is the only driver that supports RoCE LAG, and the events
> handling of mlx5 RoCE LAG will remain in mlx5 driver.
> 
> In this series:
> 
> Patch #1 adds a new helper to query the port num of a netdev
> associated with an ibdev. This is used in the following patch.
> 
> Patch #2 adds support for link status events dispatching in ib_core.
> 
> Patch #3-#7 removes link status event handler in several drivers.
> The port state setting in erdma, rxe and siw are replaced with
> ib_get_curr_port_state(), so their handler can be totally removed.
> 
> Patch #8-#10 add support for report_port_event() ops in usnic, mlx4
> and pvrdma as their current handler cannot be perfectly replaced by
> the ib_core handler in patch #2.
> 
> Patch #11 adds a check in mlx5 that only events of RoCE LAG will be
> handled in mlx5 driver.
> 
> Patch #12 adds a fast path for link-down events dispatching in hns by
> getting notified from hns3 nic driver directly.
> 
> Yuyu Li (12):
>   RDMA/core: Add ib_query_netdev_port() to query netdev port by IB
>     device.
>   RDMA/core: Support link status events dispatching
>   RDMA/bnxt_re: Remove deliver net device event
>   RDMA/erdma: Remove deliver net device event
>   RDMA/irdma: Remove deliver net device event
>   RDMA/rxe: Remove deliver net device event
>   RDMA/siw: Remove deliver net device event
>   RDMA/usnic: Support report_port_event() ops
>   RDMA/mlx4: Support report_port_event() ops
>   RDMA/pvrdma: Support report_port_event() ops
>   RDMA/mlx5: Handle link status event only for LAG device
>   RDMA/hns: Support fast path for link-down events dispatching

I took the series as it is good thing to remove code duplication
and we waited enough.

However, I'm disappointed to see "RDMA/hns: Support fast path for
link-down events dispatching" patch and would like you to use
netdev notifiers instead. I doubt that this "fast-path" is needed.

Thanks

Powered by blists - more mailing lists