> -----Original Message-----
> From: Nikolova, Tatyana E <tatyana.e.nikolova@xxxxxxxxx>
> Sent: Friday, January 25, 2019 9:58 PM
> To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>; dledford@xxxxxxxxxx;
> jgg@xxxxxxxxxxxx
> Cc: linux-rdma@xxxxxxxxxxxxxxx; BMT@xxxxxxxxxxxxxx; Saleem, Shiraz
> <shiraz.saleem@xxxxxxxxx>; 'Jason Gunthorpe' <jgg@xxxxxxxxxxxx>
> Subject: RE: [PATCH RESEND v1 rdma-next 3/3] RDMA/IWPM: Support no
> port mapping requirements
>
>
> > > Hi Steve,
> > >
> > > Your port mapper changes look good. I just have some minor comments.
> > >
> >
> > Thanks for reviewing!
> >
> > > 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.
>
> > > > +/* iwarp port mapper message flags */ enum {
> > > > +
> > > > + /* Do not map the port for this IWPM request */
> > > > + IWPM_FLAGS_NO_PORT_MAP = (1 << 0),
> > >
> > > Use BIT(0) for consistency within the patch
> > >
> >
> > I've seen other maintainers suggest BIT() is obfuscating. Jason?
> >
>
> In another place in the patch you are using the BIT() macro, so my comment
> here is just a matter of consistency.
> I don't have a strong opinion on which approach you choose.
>
I should make it consistent one way or the other.
Thanks,
Steve.
