On Mon, Jan 28, 2019 at 10:22:26AM -0600, Steve Wise wrote:
>
> On 1/27/2019 10:58 AM, Nikolova, Tatyana E wrote:
> >
> >>>>> It appears the port mapping will fail, if the kernel port mapper is updated
> >>>>> with your changes but the user space port mapper isn't (ABI
> >>>>> version 3). Is there a reason why the error is silent in this case?
> >>>>>
> >>>> With testing, I see it works with kernel ABI4 and user ABI3. How do you
> >>>> see it failing exactly? Maybe I'm missing something.
> >>>>
> >>> I am referring to the following case, which anticipates mismatch
> >>> between the kernel and user space port mapper:
> >>>
> >>>> + /* If flags are required and we're not V4, then return a quite
> >>>> +error
> >> */
> >>>> + if (pm_msg->flags && iwpm_user_ulib_version ==
> >>>> IWPM_UABI_VERSION_MIN) {
> >>>> + ret = -EINVAL;
> >>>> + goto query_mapping_error_nowarn;
> >>>> + }
> >>>> + if (iwpm_user_ulib_version > IWPM_UABI_VERSION_MIN) {
> >>>> + ret = ibnl_put_attr(skb, nlh, sizeof(u32), &pm_msg->flags,
> >>>> + IWPM_NLA_QUERY_FLAGS);
> >>>> + if (ret)
> >>>> + goto query_mapping_error;
> >>>> + }
> >> Right. So if you have a V4 kernel and a V3 iwpmd, then current iwarp drivers
> >> will work fine because they aren't trying to use the new NO_PORT_MAP flag
> >> in pm_msg->flags.
> >>
> >> If, however, you have, say the soft iwarp driver running, then pm_msg->flags
> >> would be non zero. If this is the case, then the V4 kernel must not send up
> >> the V4 FLAGS nlattr because the V3 iwpmd can't handle it.
> >> So the above code fails the map request and this results in no port mapping
> >> done for the soft iwarp driver.
> >>
> >> So we get backward compatibility with existing iwarp drivers. But new drivers
> >> that need NO_PORT_MAP will only work in the port mapping game if their
> >> local iwpmd is V4.
> >>
> > Hi Steve,
> >
> > Thank you for the explanation.
> >
> > I think that you don't need to make the error silent in this case. You could go through the standard error path, where the error is logged and people are going to know about it. This is going to prompt them to update to a more current rdma-core which has your changes.
> >
> > I understand that the error doesn't matter in the case of soft iwarp devices talking to each other, since they don't need a port mapper. However, in the case of a soft iwarp device talking to an iwarp peer which uses the port mapper, the error reporting is going to be useful, since the connection establishment won't succeed.
> >
> > Regards,
> > Tatyana
>
> That makes sense. But I only want a warn_once() type of warning.
> Thanks for reviewing this. I'll respin it soon.
dev_warn_once and print the pid/process name of the offending process
is the usual way to do this.
Also, please send the rdma-core patches, they need to come before the
kernel patch to the user api is applied
Jason
