> On Wed, Mar 07, 2018 at 11:52:28PM +0000, Hefty, Sean wrote:
> > > > diff --git a/drivers/infiniband/core/cma.c
> > > > b/drivers/infiniband/core/cma.c index 6294a70..bb8f0e2 100644
> > > > +++ b/drivers/infiniband/core/cma.c
> > > > @@ -1047,6 +1047,8 @@ EXPORT_SYMBOL(rdma_init_qp_attr);
> static
> > > inline
> > > > int cma_zero_addr(struct sockaddr *addr) {
> > > > switch (addr->sa_family) {
> > > > + case AF_UNSPEC:
> > > > + return 1;
> > >
> > > I can't figure out why this is here?
> > >
> > > I thought an 'unspecified destination' shouldn't be AF_UNSPEC.
> It is
> > > AF_INETxx and a zero address in CMA? Where can AF_UNSPEC be set
> that
> > > trips this up?
> >
> > At the time rdma_bind_addr() is called, the destination address of
> the
> > rdma_cm_id is unset (sa_family = UNSPEC). The daddr->sa_family
> isn't
> > set until the end of bind.
>
> This is surprising to me. You can't bind without also specifying a
> family, so why should be family be left as UNSPEC for so long?
rdma_bind_addr() has an sa_family for the *source* address. The *destination* address is just zeroed memory associated with the rdma_cm_id.
> Doesn't that just risk bugs where we mix and match AF's incorrectly?
>
> Could we get to the same place without the above case by just
> adjusting the bind flow to set the family earlier?
Yes, the sa_family for the destination address could be set at the top of the call and cleared on failure. It is currently only set at the end on success. Even with this change, I would still vote to make the code more consistent. If multicast join verifies the sa_family, it should be possible to assert that we're never dealing with unknown/unspecified local address formats.
- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
