[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] migration/rdma: set the REUSEADDR option for destination
From: |
Pankaj Gupta |
Subject: |
Re: [PATCH 2/2] migration/rdma: set the REUSEADDR option for destination |
Date: |
Wed, 2 Feb 2022 05:57:52 +0100 |
> > > This allow address could be reused to avoid rdma_bind_addr error
> > > out.
> >
> > Seems we are proposing to allow multiple connections on same source ip
> > port pair?
> according to the man page, it's more about the destination side which
> is the incoming side.[1]
By source here I meant generic ip port address pair binding. For this case it is
at destination live migration host.
> We hit the error on the migration target when there are many migration
> tests in parallel:
> "RDMA ERROR: Error: could not rdma_bind_addr!"
o.k. Worth to add this in commit message.
>
> [1]https://manpages.debian.org/testing/librdmacm-dev/rdma_set_option.3.en.html
> > >
> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > ---
> > > migration/rdma.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > index 2e223170d06d..b498ef013c77 100644
> > > --- a/migration/rdma.c
> > > +++ b/migration/rdma.c
> > > @@ -2705,6 +2705,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma,
> > > Error **errp)
> > > char ip[40] = "unknown";
> > > struct rdma_addrinfo *res, *e;
> > > char port_str[16];
> > > + int reuse = 1;
> > >
> > > for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> > > rdma->wr_data[idx].control_len = 0;
> > > @@ -2740,6 +2741,12 @@ static int qemu_rdma_dest_init(RDMAContext *rdma,
> > > Error **errp)
> > > goto err_dest_init_bind_addr;
> > > }
> > >
> > > + ret = rdma_set_option(listen_id, RDMA_OPTION_ID,
> > > RDMA_OPTION_ID_REUSEADDR,
> > > + &reuse, sizeof reuse);
> >
> > maybe we can just write '1' directly on the argument list of
> > 'rdma_set_option'.
> > Assuming reuseaddr does not effect core rdma transport? change seems ok to
> > me.
> I feel it's cleaner to do it with a variable than force conversion of
> 1 to void *.
Adding typecasted 1 seems more cleaner than adding another auto variable.
Will leave this to maintainers to decide.
>
> It's bound to the cm_id which is newly created a few lines above, so
> does not affect core rdma transport.
o.k.
>
> >
> > Thanks,
> > Pankaj
> Thanks for the review!
>
> Jinpu Wang
> >
> > > + if (ret) {
> > > + ERROR(errp, "Error: could not set REUSEADDR option");
> > > + goto err_dest_init_bind_addr;
> > > + }
> > > for (e = res; e != NULL; e = e->ai_next) {
> > > inet_ntop(e->ai_family,
> > > &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip,
> > > sizeof ip);
> > > --
> > > 2.25.1
> > >
[PATCH 2/2] migration/rdma: set the REUSEADDR option for destination, Jack Wang, 2022/02/01
Re: [PATCH 2/2] migration/rdma: set the REUSEADDR option for destination, Dr. David Alan Gilbert, 2022/02/02