Message ID | 20150420200111.GA32449@obsidianresearch.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On 20/04/2015 23:01, Jason Gunthorpe wrote: > On Mon, Apr 20, 2015 at 09:38:02PM +0300, Or Gerlitz wrote: >> On Mon, Apr 20, 2015 at 7:41 PM, Jason Gunthorpe >> <jgunthorpe@obsidianresearch.com> wrote: >>> On Mon, Apr 20, 2015 at 12:03:32PM +0300, Haggai Eran wrote: >>>> From: Yotam Kenneth <yotamke@mellanox.com> >>>> >>>> When accepting a new connection with the listener being IPv6, the >>>> family of the new connection is set as IPv6. This causes cma_zero_addr >>>> function to return true on an non-zero address. As a result, the wrong >>>> code path is taken. This causes the connection request to be rejected, >>>> as the RDMA-CM code looks for the wrong type of device. >>> >>> This description doesn't really make sense as to what the problem is. > >> Jason, could you take a look @ this thread >> http://marc.info/?t=141589395000004&r=1&w=2 where the authors >> addressed some comments from Sean and he eventually Acked the patch? > > Please actually read my comments: > > If listen_id->route.addr.src_addr.ss_family != AF_INET then it is > invalid to cast to sockaddr_in. That's correct. We didn't address it because it was part of the existing code. Anyway, in a later patch in this series we move this code from the CMA to the CM module. Then we get the port number from the service ID instead of from the listener ID, since the listener ID's port isn't available. > > Sean asked basically the same thing, and his question was ignored too. > > This should take care of it, testing, and figuring the fixes tag is > left as an exercise to the reader.. > Fixes: e51060f08a61 ("IB: IP address based RDMA connection manager") Tested-by: Haggai Eran <haggaie@mellanox.com> Haggai -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, April 21, 2015 1:15 PM, Haggai Eran <haggaie@mellanox.com> wrote: > On 20/04/2015 23:01, Jason Gunthorpe wrote: >> This should take care of it, testing, and figuring the fixes tag is >> left as an exercise to the reader.. > > Fixes: e51060f08a61 ("IB: IP address based RDMA connection manager") > Tested-by: Haggai Eran <haggaie@mellanox.com> > Roland, Doug, Could you pick Jason's patch without the rest of the series? It seems the namespace series will need more work, but I don't think this patch should be delayed as well. Thanks, Haggai-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 21, 2015 at 01:15:11PM +0300, Haggai Eran wrote: > That's correct. We didn't address it because it was part of the existing > code. Anyway, in a later patch in this series we move this code from the > CMA to the CM module. Just so we are all on the same page in the future: - Don't half fix bugs: 'part of the existing code' is not an excuse. - Don't mix clearly independent bug fixes into a patch series. > Fixes: e51060f08a61 ("IB: IP address based RDMA connection manager") > Tested-by: Haggai Eran <haggaie@mellanox.com> Thanks Jason -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index d570030d899c..e8d492eceff3 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -859,19 +859,27 @@ static void cma_save_ib_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id memcpy(&ib->sib_addr, &path->dgid, 16); } +static unsigned int ss_get_port(const struct sockaddr_storage *ss) +{ + if (ss->ss_family == AF_INET) + return ((struct sockaddr_in *)ss)->sin_port; + else if (ss->ss_family == AF_INET6) + return ((struct sockaddr_in6 *)ss)->sin6_port; + BUG(); +} + static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id, struct cma_hdr *hdr) { - struct sockaddr_in *listen4, *ip4; + struct sockaddr_in *ip4; - listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr; ip4 = (struct sockaddr_in *) &id->route.addr.src_addr; - ip4->sin_family = listen4->sin_family; + ip4->sin_family = AF_INET; ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr; - ip4->sin_port = listen4->sin_port; + ip4->sin_port = ss_get_port(&listen_id->route.addr.src_addr); ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr; - ip4->sin_family = listen4->sin_family; + ip4->sin_family = AF_INET; ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr; ip4->sin_port = hdr->port; } @@ -879,16 +887,15 @@ static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i static void cma_save_ip6_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id, struct cma_hdr *hdr) { - struct sockaddr_in6 *listen6, *ip6; + struct sockaddr_in6 *ip6; - listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr; ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr; - ip6->sin6_family = listen6->sin6_family; + ip6->sin6_family = AF_INET6; ip6->sin6_addr = hdr->dst_addr.ip6; - ip6->sin6_port = listen6->sin6_port; + ip6->sin6_port = ss_get_port(&listen_id->route.addr.src_addr); ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr; - ip6->sin6_family = listen6->sin6_family; + ip6->sin6_family = AF_INET6; ip6->sin6_addr = hdr->src_addr.ip6; ip6->sin6_port = hdr->port; }