On Sat, Jun 01, 2024 at 12:48:40PM +0200, Zhu Yanjun wrote:
> On 31.05.24 15:15, cel@xxxxxxxxxx wrote:
> > From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >
> > Sagi tells me that when a bonded device reports an address change,
> > the consumer must destroy its listener IDs and create new ones.
> >
> > See commit a032e4f6d60d ("nvmet-rdma: fix bonding failover possible
> > NULL deref").
> >
> > Suggested-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
> > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > ---
> > net/sunrpc/xprtrdma/svc_rdma_transport.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > index fa50b7494a0a..a003b62fb7d5 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -284,17 +284,31 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id,
> > *
> > * Return values:
> > * %0: Do not destroy @cma_id
> > - * %1: Destroy @cma_id (never returned here)
> > + * %1: Destroy @cma_id
> > *
> > * NB: There is never a DEVICE_REMOVAL event for INADDR_ANY listeners.
> > */
> > static int svc_rdma_listen_handler(struct rdma_cm_id *cma_id,
> > struct rdma_cm_event *event)
> > {
> > + struct svcxprt_rdma *cma_xprt = cma_id->context;
> > + struct svc_xprt *cma_rdma = &cma_xprt->sc_xprt;
> > + struct sockaddr *sap = (struct sockaddr *)&cma_id->route.addr.src_addr;
> > + struct rdma_cm_id *listen_id;
>
> I am not sure whether I need to suggest "Reverse Christmas Tree" or not.
> This is a trivial problem. ^_^
> You can ignore it. And it is not mandatory.
Oops, I missed that @sap can be declared first. It must have gotten
lost while I was trying out different solutions for this issue.
Fixed.
> But if we follow this rule, the source code will become more readable.
>
> Zhu Yanjun
>
> > +
> > switch (event->event) {
> > case RDMA_CM_EVENT_CONNECT_REQUEST:
> > handle_connect_req(cma_id, &event->param.conn);
> > break;
> > + case RDMA_CM_EVENT_ADDR_CHANGE:
> > + listen_id = svc_rdma_create_listen_id(cma_rdma->xpt_net,
> > + sap, cma_xprt);
> > + if (IS_ERR(listen_id)) {
> > + pr_err("Listener dead, address change failed for device %s\n",
> > + cma_id->device->name);
> > + } else
> > + cma_xprt->sc_cm_id = listen_id;
> > + return 1;
> > default:
> > break;
> > }
>
--
Chuck Lever
