On Fri, Feb 14, 2025 at 10:43 AM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
> On 2/14/25 10:38 AM, Olga Kornievskaia wrote:
> > On Fri, Feb 14, 2025 at 9:24 AM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> >>
> >> On 2/13/25 12:30 PM, Olga Kornievskaia wrote:
> >>> On Thu, Feb 13, 2025 at 11:01 AM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> >>>>
> >>>> On 2/13/25 10:47 AM, Olga Kornievskaia wrote:
> >>>>> Don't ignore return code of adding rdma listener. If nfs.conf has asked
> >>>>> for "rdma=y" but adding the listener fails, don't ignore the failure.
> >>>>> Note in soft-rdma-provider environment (such as soft iwarp, soft roce),
> >>>>> when no address-constraints are used, an "any" listener is created and
> >>>>> rdma-enabling is done independently.
> >>>>
> >>>> This behavior is confusing... I suggest that an nfs.conf man page
> >>>> update accompany the below code change.
> >>>
> >>> Do you find only the rdma=y soft-rdma case confusing, or do you find
> >>> that when listeners fail and we shouldn't start knfsd threads in
> >>> general confusing?
> >>>
> >>> It was always the case that if rdma=y is done, then any listener
> >>> created for it does not check whether or not the underlying interface
> >>> is already rdma-enabled. This hasn't changed. Nor does this patch
> >>> change it.
> >>
> >> Not saying the patch changes the behavior. But you have to admit the
> >> behavior is surprising and needs clear documentation.
> >
> > Sure we can document the behavior of the any listener on a soft rdma
> > interface as it's used by the knfsd. But is it guaranteed not to
> > change, as the behaviour is controlled by the RDMA core not NFS?
>
> I'm talking about documenting the opposite. Something like:
>
> When "host=" is present, the network interface named by this
> configuration setting must be present (and when "rdma=y", that
> device must be RDMA-enabled) in order for server start-up to
> succeed.
Ah that case. Yes, I agree.
> >>>> Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >>>>
> >>>>
> >>>>> Fixes: e3b72007ab31 ("nfs-utils: nfsdctl: cleanup listeners if some failed")
> >>>>> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> >>>>> ---
> >>>>> utils/nfsdctl/nfsdctl.c | 4 ++--
> >>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> >>>>> index 05fecc71..244910ef 100644
> >>>>> --- a/utils/nfsdctl/nfsdctl.c
> >>>>> +++ b/utils/nfsdctl/nfsdctl.c
> >>>>> @@ -1388,7 +1388,7 @@ static int configure_listeners(void)
> >>>>> if (tcp)
> >>>>> ret = add_listener("tcp", n->field, port);
> >>>>> if (rdma)
> >>>>> - add_listener("rdma", n->field, rdma_port);
> >>>>> + ret = add_listener("rdma", n->field, rdma_port);
> >>>>> if (ret)
> >>>>> return ret;
> >>>>> }
> >>>>> @@ -1398,7 +1398,7 @@ static int configure_listeners(void)
> >>>>> if (tcp)
> >>>>> ret = add_listener("tcp", "", port);
> >>>>> if (rdma)
> >>>>> - add_listener("rdma", "", rdma_port);
> >>>>> + ret = add_listener("rdma", "", rdma_port);
> >>>>> }
> >>>>> return ret;
> >>>>> }
> >>>>
> >>>>
> >>>> --
> >>>> Chuck Lever
> >>>>
> >>>
> >>
> >>
> >> --
> >> Chuck Lever
> >>
>
>
> --
> Chuck Lever
