| Message ID | 20231221213358.105704-9-aaptel@nvidia.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | nvme-tcp receive offloads | expand |
On 12/21/23 23:33, Aurelien Aptel wrote: > From: Or Gerlitz <ogerlitz@nvidia.com> > > For ddp setup/teardown and resync, the offloading logic > uses HW resources at the NIC driver such as SQ and CQ. > > These resources are destroyed when the netdevice does down > and hence we must stop using them before the NIC driver > destroys them. > > Use netdevice notifier for that matter -- offloaded connections > are stopped before the stack continues to call the NIC driver > close ndo. > > We use the existing recovery flow which has the advantage > of resuming the offload once the connection is re-set. > > This also buys us proper handling for the UNREGISTER event > b/c our offloading starts in the UP state, and down is always > there between up to unregister. > > Signed-off-by: Or Gerlitz <ogerlitz@nvidia.com> > Signed-off-by: Boris Pismenny <borisp@nvidia.com> > Signed-off-by: Ben Ben-Ishay <benishay@nvidia.com> > Signed-off-by: Yoray Zack <yorayz@nvidia.com> > Signed-off-by: Shai Malin <smalin@nvidia.com> > Signed-off-by: Aurelien Aptel <aaptel@nvidia.com> > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/nvme/host/tcp.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c > index 6eed24b5f90c..00cb1c8404c4 100644 > --- a/drivers/nvme/host/tcp.c > +++ b/drivers/nvme/host/tcp.c > @@ -235,6 +235,7 @@ struct nvme_tcp_ctrl { > > static LIST_HEAD(nvme_tcp_ctrl_list); > static DEFINE_MUTEX(nvme_tcp_ctrl_mutex); > +static struct notifier_block nvme_tcp_netdevice_nb; > static struct workqueue_struct *nvme_tcp_wq; > static const struct blk_mq_ops nvme_tcp_mq_ops; > static const struct blk_mq_ops nvme_tcp_admin_mq_ops; > @@ -3193,6 +3194,32 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev, > return ERR_PTR(ret); > } > > +static int nvme_tcp_netdev_event(struct notifier_block *this, > + unsigned long event, void *ptr) > +{ > +#ifdef CONFIG_ULP_DDP > + struct net_device *ndev = netdev_notifier_info_to_dev(ptr); > + struct nvme_tcp_ctrl *ctrl; > + > + switch (event) { > + case NETDEV_GOING_DOWN: > + mutex_lock(&nvme_tcp_ctrl_mutex); > + list_for_each_entry(ctrl, &nvme_tcp_ctrl_list, list) { > + if (ndev == ctrl->ddp_netdev) > + nvme_tcp_error_recovery(&ctrl->ctrl); > + } > + mutex_unlock(&nvme_tcp_ctrl_mutex); > + flush_workqueue(nvme_reset_wq); > + /* > + * The associated controllers teardown has completed, > + * ddp contexts were also torn down so we should be > + * safe to continue... > + */ > + } Will this handler ever react to another type of event? because if not, maybe its better to just have: if (event != NETDEV_GOING_DOWN) return NOTIFY_DONE; ... > +#endif > + return NOTIFY_DONE; > +} > + > static struct nvmf_transport_ops nvme_tcp_transport = { > .name = "tcp", > .module = THIS_MODULE, > @@ -3208,6 +3235,8 @@ static struct nvmf_transport_ops nvme_tcp_transport = { > > static int __init nvme_tcp_init_module(void) > { > + int ret; > + > BUILD_BUG_ON(sizeof(struct nvme_tcp_hdr) != 8); > BUILD_BUG_ON(sizeof(struct nvme_tcp_cmd_pdu) != 72); > BUILD_BUG_ON(sizeof(struct nvme_tcp_data_pdu) != 24); > @@ -3222,8 +3251,19 @@ static int __init nvme_tcp_init_module(void) > if (!nvme_tcp_wq) > return -ENOMEM; > > + nvme_tcp_netdevice_nb.notifier_call = nvme_tcp_netdev_event; > + ret = register_netdevice_notifier(&nvme_tcp_netdevice_nb); > + if (ret) { > + pr_err("failed to register netdev notifier\n"); > + goto out_free_workqueue; > + } > + > nvmf_register_transport(&nvme_tcp_transport); > return 0; > + > +out_free_workqueue: > + destroy_workqueue(nvme_tcp_wq); > + return ret; > } > > static void __exit nvme_tcp_cleanup_module(void) > @@ -3231,6 +3271,7 @@ static void __exit nvme_tcp_cleanup_module(void) > struct nvme_tcp_ctrl *ctrl; > > nvmf_unregister_transport(&nvme_tcp_transport); > + unregister_netdevice_notifier(&nvme_tcp_netdevice_nb); > > mutex_lock(&nvme_tcp_ctrl_mutex); > list_for_each_entry(ctrl, &nvme_tcp_ctrl_list, list)
Sagi Grimberg <sagi@grimberg.me> writes: > > Will this handler ever react to another type of event? because > if not, maybe its better to just have: > > if (event != NETDEV_GOING_DOWN) > return NOTIFY_DONE; > > ... Hi Sagi, This patch was already reviewed-by you. If you wish we can change the switch to an if. Any other comments on other patches? Thanks
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 6eed24b5f90c..00cb1c8404c4 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -235,6 +235,7 @@ struct nvme_tcp_ctrl { static LIST_HEAD(nvme_tcp_ctrl_list); static DEFINE_MUTEX(nvme_tcp_ctrl_mutex); +static struct notifier_block nvme_tcp_netdevice_nb; static struct workqueue_struct *nvme_tcp_wq; static const struct blk_mq_ops nvme_tcp_mq_ops; static const struct blk_mq_ops nvme_tcp_admin_mq_ops; @@ -3193,6 +3194,32 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev, return ERR_PTR(ret); } +static int nvme_tcp_netdev_event(struct notifier_block *this, + unsigned long event, void *ptr) +{ +#ifdef CONFIG_ULP_DDP + struct net_device *ndev = netdev_notifier_info_to_dev(ptr); + struct nvme_tcp_ctrl *ctrl; + + switch (event) { + case NETDEV_GOING_DOWN: + mutex_lock(&nvme_tcp_ctrl_mutex); + list_for_each_entry(ctrl, &nvme_tcp_ctrl_list, list) { + if (ndev == ctrl->ddp_netdev) + nvme_tcp_error_recovery(&ctrl->ctrl); + } + mutex_unlock(&nvme_tcp_ctrl_mutex); + flush_workqueue(nvme_reset_wq); + /* + * The associated controllers teardown has completed, + * ddp contexts were also torn down so we should be + * safe to continue... + */ + } +#endif + return NOTIFY_DONE; +} + static struct nvmf_transport_ops nvme_tcp_transport = { .name = "tcp", .module = THIS_MODULE, @@ -3208,6 +3235,8 @@ static struct nvmf_transport_ops nvme_tcp_transport = { static int __init nvme_tcp_init_module(void) { + int ret; + BUILD_BUG_ON(sizeof(struct nvme_tcp_hdr) != 8); BUILD_BUG_ON(sizeof(struct nvme_tcp_cmd_pdu) != 72); BUILD_BUG_ON(sizeof(struct nvme_tcp_data_pdu) != 24); @@ -3222,8 +3251,19 @@ static int __init nvme_tcp_init_module(void) if (!nvme_tcp_wq) return -ENOMEM; + nvme_tcp_netdevice_nb.notifier_call = nvme_tcp_netdev_event; + ret = register_netdevice_notifier(&nvme_tcp_netdevice_nb); + if (ret) { + pr_err("failed to register netdev notifier\n"); + goto out_free_workqueue; + } + nvmf_register_transport(&nvme_tcp_transport); return 0; + +out_free_workqueue: + destroy_workqueue(nvme_tcp_wq); + return ret; } static void __exit nvme_tcp_cleanup_module(void) @@ -3231,6 +3271,7 @@ static void __exit nvme_tcp_cleanup_module(void) struct nvme_tcp_ctrl *ctrl; nvmf_unregister_transport(&nvme_tcp_transport); + unregister_netdevice_notifier(&nvme_tcp_netdevice_nb); mutex_lock(&nvme_tcp_ctrl_mutex); list_for_each_entry(ctrl, &nvme_tcp_ctrl_list, list)