| Message ID | 20230712161513.134860-12-aaptel@nvidia.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | nvme-tcp receive offloads | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply, async |
On 7/12/23 19:14, Aurelien Aptel wrote: > Add ulp_offload module parameter to the nvme-tcp module to control > ULP offload at the NVMe-TCP layer. > > Turn ULP offload off be default, regardless of the NIC driver support. > > Overall, in order to enable ULP offload: > - nvme-tcp ulp_offload modparam must be set to 1 > - netdev->ulp_ddp_caps.active must have ULP_DDP_C_NVME_TCP and/or > ULP_DDP_C_NVME_TCP_DDGST_RX capabilities flag set. > > Signed-off-by: Yoray Zack <yorayz@nvidia.com> > Signed-off-by: Shai Malin <smalin@nvidia.com> > Signed-off-by: Aurelien Aptel <aaptel@nvidia.com> > --- > drivers/nvme/host/tcp.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c > index e68e5da3df76..e560bdf3a023 100644 > --- a/drivers/nvme/host/tcp.c > +++ b/drivers/nvme/host/tcp.c > @@ -49,6 +49,16 @@ MODULE_PARM_DESC(tls_handshake_timeout, > "nvme TLS handshake timeout in seconds (default 10)"); > #endif > > +#ifdef CONFIG_ULP_DDP > +/* NVMeTCP direct data placement and data digest offload will not > + * happen if this parameter false (default), regardless of what the > + * underlying netdev capabilities are. > + */ > +static bool ulp_offload; > +module_param(ulp_offload, bool, 0644); > +MODULE_PARM_DESC(ulp_offload, "Enable or disable NVMeTCP ULP support"); the name is strange. maybe call it ddp_offload? and in the description spell it as "direct data placement" > +#endif > + > #ifdef CONFIG_DEBUG_LOCK_ALLOC > /* lockdep can detect a circular dependency of the form > * sk_lock -> mmap_lock (page fault) -> fs locks -> sk_lock > @@ -350,7 +360,7 @@ static bool nvme_tcp_ddp_query_limits(struct net_device *netdev, > static inline bool is_netdev_ulp_offload_active(struct net_device *netdev, > struct nvme_tcp_queue *queue) > { > - if (!netdev || !queue) > + if (!ulp_offload || !netdev || !queue) > return false; > > /* If we cannot query the netdev limitations, do not offload */ This patch should be folded to the control path. No reason for it to stand on its own I think.
Sagi Grimberg <sagi@grimberg.me> writes: >> +static bool ulp_offload; >> +module_param(ulp_offload, bool, 0644); >> +MODULE_PARM_DESC(ulp_offload, "Enable or disable NVMeTCP ULP support"); > > the name is strange. > maybe call it ddp_offload? > and in the description spell it as "direct data placement" Sure. > This patch should be folded to the control path. No reason for it to > stand on its own I think. Sure, we will fold it. Thanks
On 09/08/2023 11:03, Sagi Grimberg wrote: > > > On 7/12/23 19:14, Aurelien Aptel wrote: >> Add ulp_offload module parameter to the nvme-tcp module to control >> ULP offload at the NVMe-TCP layer. >> >> Turn ULP offload off be default, regardless of the NIC driver support. >> >> Overall, in order to enable ULP offload: >> - nvme-tcp ulp_offload modparam must be set to 1 >> - netdev->ulp_ddp_caps.active must have ULP_DDP_C_NVME_TCP and/or >> ULP_DDP_C_NVME_TCP_DDGST_RX capabilities flag set. >> >> Signed-off-by: Yoray Zack <yorayz@nvidia.com> >> Signed-off-by: Shai Malin <smalin@nvidia.com> >> Signed-off-by: Aurelien Aptel <aaptel@nvidia.com> >> --- >> drivers/nvme/host/tcp.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c >> index e68e5da3df76..e560bdf3a023 100644 >> --- a/drivers/nvme/host/tcp.c >> +++ b/drivers/nvme/host/tcp.c >> @@ -49,6 +49,16 @@ MODULE_PARM_DESC(tls_handshake_timeout, >> "nvme TLS handshake timeout in seconds (default 10)"); >> #endif >> +#ifdef CONFIG_ULP_DDP >> +/* NVMeTCP direct data placement and data digest offload will not >> + * happen if this parameter false (default), regardless of what the >> + * underlying netdev capabilities are. >> + */ >> +static bool ulp_offload; >> +module_param(ulp_offload, bool, 0644); >> +MODULE_PARM_DESC(ulp_offload, "Enable or disable NVMeTCP ULP support"); > > the name is strange. > maybe call it ddp_offload? Agree. ddp_offload is a better fit. > and in the description spell it as "direct data placement" > >> +#endif >> + >> #ifdef CONFIG_DEBUG_LOCK_ALLOC >> /* lockdep can detect a circular dependency of the form >> * sk_lock -> mmap_lock (page fault) -> fs locks -> sk_lock >> @@ -350,7 +360,7 @@ static bool nvme_tcp_ddp_query_limits(struct >> net_device *netdev, >> static inline bool is_netdev_ulp_offload_active(struct net_device >> *netdev, >> struct nvme_tcp_queue *queue) >> { >> - if (!netdev || !queue) >> + if (!ulp_offload || !netdev || !queue) >> return false; >> /* If we cannot query the netdev limitations, do not offload */ > > This patch should be folded to the control path. No reason for it to > stand on its own I think. Agree.
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index e68e5da3df76..e560bdf3a023 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -49,6 +49,16 @@ MODULE_PARM_DESC(tls_handshake_timeout, "nvme TLS handshake timeout in seconds (default 10)"); #endif +#ifdef CONFIG_ULP_DDP +/* NVMeTCP direct data placement and data digest offload will not + * happen if this parameter false (default), regardless of what the + * underlying netdev capabilities are. + */ +static bool ulp_offload; +module_param(ulp_offload, bool, 0644); +MODULE_PARM_DESC(ulp_offload, "Enable or disable NVMeTCP ULP support"); +#endif + #ifdef CONFIG_DEBUG_LOCK_ALLOC /* lockdep can detect a circular dependency of the form * sk_lock -> mmap_lock (page fault) -> fs locks -> sk_lock @@ -350,7 +360,7 @@ static bool nvme_tcp_ddp_query_limits(struct net_device *netdev, static inline bool is_netdev_ulp_offload_active(struct net_device *netdev, struct nvme_tcp_queue *queue) { - if (!netdev || !queue) + if (!ulp_offload || !netdev || !queue) return false; /* If we cannot query the netdev limitations, do not offload */