On 21/5/2014 11:00 PM, Jason Gunthorpe wrote:
On Sun, May 18, 2014 at 12:39:11PM +0300, Or Gerlitz wrote:
From: Matan Barak <[email protected]>

In order to implement IP based addressing for UD QPs, we need a way to
resolve the addresses internally.
The L2 params are passed to the provider driver using an extension verbs
- drv_ibv_create_ah_ex.
    ^^^^^^
The name changed


Thanks!

+struct ibv_ah *mlx4_create_ah_ex(struct ibv_pd *pd,
+                                struct ibv_ah_attr_ex *attr_ex)
+{
+       struct ibv_port_attr port_attr;
+       struct ibv_ah *ah;
+       struct mlx4_ah *mah;
+
+       if (ibv_query_port(pd->context, attr_ex->port_num, &port_attr))
+               return NULL;
+
+       ah = mlx4_create_ah_common(pd, (struct ibv_ah_attr *)attr_ex,
+                                  port_attr.link_layer);
+
+       if (NULL == ah)
+               return NULL;

I'm seeing a real lack of error reporting here.


Correct, ibv_query_port error should be captured to errno,
In addition, we need to add an error for an incorrect link layer type.

My prior question was never answered: Should we be consistently using
errno for functions that return pointers? If yes, you need to make
sure errno is set on these failure paths, and document the possible
values errno can take.

+       /* check that ll is provided and valid */
+       if (attr_ex->comp_mask & IBV_AH_ATTR_EX_LL) {
+               if (ARPHRD_ETHER != attr_ex->ll.sa.sa_family ||
                   ^^^^^^^^^^^
As I mentioned before, ARPHDR_ETHER is not correct for sa_family.


I don't want to tie this mechanism to Ethernet. If you prefer in-lining the data (rather than using a pointer), we might just introduce a new type of a big sockaddr_storage like buffer with a link_layer type enum.
What do you think?

Jason


Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to