> > 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.
> It seems really wierd to call AF_UNSPEC 'zero'..
The check for UNSPEC can move out of cma_zero_addr() into cma_any_addr(), but that doesn't address the next concern...
> And if you change this then it means cma_any_addr() becomes true for
> AF_UNSPEC.. And that seems like it would have impact beyond just
> cma_port_is_unique,
>
> eg here:
>
> static bool cma_match_private_data(struct rdma_id_private *id_priv,
> const struct cma_hdr *hdr)
> {
> if (cma_any_addr(addr) && !id_priv->afonly)
> return true;
>
> Now AF_UNSPEC returns true, where before it was false.. Seems
> concerning.
>
> This really needs more explanation please.
The behavior of cma_any_addr() and cma_any_port() is different for UNSPEC. cma_any_addr() returns false, but cma_any_port() returns true. The latter results from cma_port() returning 0 in the case of UNSPEC (or any unhandled address format). For consistency, cma_zero_addr() was modified to align with cma_port(), which also makes any_addr/port consistent.
In most of the cases I checked, passing in an address with sa_family set to UNSPEC to cma_any_addr() should not occur. The port available check is one (valid) case where it can occur. The cma_match_private_data() call referenced above _should_ always have a valid sa_family set for the source address, for example.
The only place that it looks like cma_any_addr() might be called with sa_family = UNSPEC is cma_set_mgid()/cma_iboe_set_mgid(). And I'm not sure the existing code through those paths is correct. If the address is UNSPEC, then the existing code will default to the AF_INET case.
The librdmacm checks for UNSPEC in multicast join operations, so we may never be hitting this case in practice. But I think a user space app could invoke this code path. I think the result of that would just be the formation of a gid with 4 bytes of 0's in it. In any case, the proposed changes would modify this path and convert UNSPEC to an mgid = 0.
- 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
