>Subject: Re: [PATCH] RDMA/mana_ib: Use v2 version of cfg_rx_steer_req to >enable RX coalescing > >On Tue, May 09, 2023 at 07:08:36PM +0000, Long Li wrote: >> > Subject: Re: [PATCH] RDMA/mana_ib: Use v2 version of >> > cfg_rx_steer_req to enable RX coalescing >> > >> > On Mon, May 08, 2023 at 02:45:44PM +0000, Haiyang Zhang wrote: >> > > >> > > >> > > > -----Original Message----- >> > > > From: Leon Romanovsky <leon@xxxxxxxxxx> >> > > > Sent: Monday, May 8, 2023 2:10 AM >> > > > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> >> > > > Cc: Long Li <longli@xxxxxxxxxxxxx>; Jason Gunthorpe >> > > > <jgg@xxxxxxxx>; Ajay Sharma <sharmaajay@xxxxxxxxxxxxx>; Dexuan >> > > > Cui <decui@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>; >> > > > Wei Liu >> > <wei.liu@xxxxxxxxxx>; David S. >> > > > Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet >> > > > <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo >> > > > Abeni <pabeni@xxxxxxxxxx>; >> > > > linux- rdma@xxxxxxxxxxxxxxx; linux-hyperv@xxxxxxxxxxxxxxx; >> > > > netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx >> > > > Subject: Re: [PATCH] RDMA/mana_ib: Use v2 version of >> > > > cfg_rx_steer_req to enable RX coalescing >> > > > >> > > > On Sun, May 07, 2023 at 09:39:27PM +0000, Haiyang Zhang wrote: >> > > > > >> > > > > >> > > > > > -----Original Message----- >> > > > > > From: Leon Romanovsky <leon@xxxxxxxxxx> >> > > > > > Sent: Sunday, May 7, 2023 4:11 AM >> > > > > > To: Long Li <longli@xxxxxxxxxxxxx> >> > > > > > Cc: Jason Gunthorpe <jgg@xxxxxxxx>; Ajay Sharma >> > > > > > <sharmaajay@xxxxxxxxxxxxx>; Dexuan Cui >> > > > > > <decui@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>; >> > > > > > Haiyang Zhang >> > > > <haiyangz@xxxxxxxxxxxxx>; >> > > > > > Wei Liu <wei.liu@xxxxxxxxxx>; David S. Miller >> > > > > > <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; >> > Jakub >> > > > > > Kicinski <kuba@xxxxxxxxxx>; >> > > > Paolo >> > > > > > Abeni <pabeni@xxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx; >> > > > > > linux- hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; >> > > > > > linux- kernel@xxxxxxxxxxxxxxx >> > > > > > Subject: Re: [PATCH] RDMA/mana_ib: Use v2 version of >> > > > > > cfg_rx_steer_req >> > > > to >> > > > > > enable RX coalescing >> > > > > > >> > > > > > On Fri, May 05, 2023 at 11:51:48AM -0700, >> > > > > > longli@xxxxxxxxxxxxxxxxx >> > > > > > wrote: >> > > > > > > From: Long Li <longli@xxxxxxxxxxxxx> >> > > > > > > >> > > > > > > With RX coalescing, one CQE entry can be used to indicate >> > > > > > > multiple >> > > > packets >> > > > > > > on the receive queue. This saves processing time and PCI >> > > > > > > bandwidth over the CQ. >> > > > > > > >> > > > > > > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx> >> > > > > > > --- >> > > > > > > drivers/infiniband/hw/mana/qp.c | 5 ++++- >> > > > > > > include/net/mana/mana.h | 17 +++++++++++++++++ >> > > > > > > 2 files changed, 21 insertions(+), 1 deletion(-) >> > > > > > >> > > > > > Why didn't you change mana_cfg_vport_steering() too? >> > > > > >> > > > > The mana_cfg_vport_steering() is for mana_en (Enthernet) >> > > > > driver, not the mana_ib driver. >> > > > > >> > > > > The changes for mana_en will be done in a separate patch >> > > > > together with changes for mana_en RX code patch to support >> > > > > multiple packets / >> > CQE. >> > > > >> > > > I'm aware of the difference between mana_en and mana_ib. >> > > > >> > > > The change you proposed doesn't depend on "support multiple >> > > > packets / CQE." >> > > > and works perfectly with one packet/CQE also, does it? >> > > >> > > No. >> > > If we add the following setting to the mana_en / >> > > mana_cfg_vport_steering(), the NIC may put multiple packets in one >> > > CQE, so we need to have the changes for mana_en RX code path to >> > > support >> > multiple packets / CQE. >> > > + req->cqe_coalescing_enable = true; >> > >> > You can leave "cqe_coalescing_enable = false" for ETH and still >> > reuse your new >> > v2 struct. >> >> I think your proposal will work for both Ethernet and IB. >> >> The idea is that we want this patch to change the behavior of the IB driver. We >plan to make another patch for the Ethernet driver. This makes it easier to track >all changes for a driver. > >And I don't want to deal with deletion of v1 struct for two/three kernel cycles >instead of one patch in one cycle. I'm resubmitting this patch to replace v1 for both driver. > >> >> > >> > H> >> > > So we plan to set this cqe_coalescing_enable, and the changes for >> > > mana_en RX code path to support multiple packets / CQE in another >patch. >> > >> > And how does it work with IB without changing anything except this >> > proposed patch? >> >> The RX CQE Coalescing is implemented in the user-mode. This feature is >always turned on from cluster. The user-mode code is written in a way that can >deal with both CQE Coalescing and CQE non-coalescing, so it doesn't depend on >kernel version for the correct behavior. > >Yes, but how does userspace know that CQE coalescing was enabled? The user-mode doesn't know if CQE is enabled in advance. If this information is required I can modify the patch to pass this information along to rdma-core. However, this is not useful as the cluster is running with CQE coalescing enabled by default, there is no need to know this information.
