[ofw] patch 2/2 Add support for RDMAoEth to the low level driver

[Sean Hefty]

>Index: core/al/kernel/al_cm_cep.c
>===================================================================
>--- core/al/kernel/al_cm_cep.c (revision 2617)
>+++ core/al/kernel/al_cm_cep.c (working copy)
>@@ -4390,6 +4390,10 @@
>
>  p_av->attr.grh_valid = !ib_gid_is_link_local( &p_path->dgid );
>
>+ // make this same as Linux code: p_path->hop_flow_raw >= 2 means grh present.
>+ if(cl_hton32(p_path->hop_flow_raw) >= 2)
>+  p_av->attr.grh_valid = 1;

Please submit this as a separate patch, and it would be best to decide on a
single way to set grh_valid.


>Index: ulp/libibverbs/examples/pingpong.c
>===================================================================
>--- ulp/libibverbs/examples/pingpong.c (revision 2617)
>+++ ulp/libibverbs/examples/pingpong.c (working copy)
>@@ -50,3 +50,25 @@
>
>  return attr.lid;
> }
>+
>+void str2gid(char *grh, union ibv_gid *gid)
>+{
>+ char tmp;
>+
>+ tmp = grh[8];
>+ grh[8] = 0;
>+ (*((ULONG*)&gid->raw[0])) = htonl(strtoul(grh, NULL, 16));
>+ grh[8] = tmp;
>+
>+ tmp = grh[16];
>+ grh[16] = 0;
>+ (*((ULONG*)&gid->raw[4])) = htonl(strtoul(grh + 8, NULL, 16));
>+ grh[16] = tmp;
>+
>+ tmp = grh[24];
>+ grh[24] = 0;
>+ (*((ULONG*)&gid->raw[8])) = htonl(strtoul(grh + 16, NULL, 16));
>+ grh[24] = tmp;
>+
>+ (*((ULONG*)&gid->raw[12])) = htonl(strtoul(grh + 24, NULL, 16));
>+}

This should just use inet_pton, and please submit libibverb changes as a
separate patch.

Other QP connection parameters (QPN, LID) are automatically discovered and
exchanged by the libibverbs sample programs.  Why have the user specify this one
value, rather than letting each side discover the GIDs using a local query?

> #endif /* IBV_PINGPONG_H */
>Index: ulp/libibverbs/examples/rc_pingpong/rc_pingpong.c
>===================================================================
>--- ulp/libibverbs/examples/rc_pingpong/rc_pingpong.c (revision 2617)
>+++ ulp/libibverbs/examples/rc_pingpong/rc_pingpong.c (working copy)
>@@ -44,6 +44,8 @@
>  PINGPONG_SEND_WRID = 2,
> };
>
>+char           *grh = NULL;
>+
> struct pingpong_context {
>  struct ibv_context  *context;
>  struct ibv_comp_channel *channel;
>@@ -55,6 +57,8 @@
>  int      size;
>  int      rx_depth;
>  int      pending;
>+ struct ibv_port_attr     portinfo;

I don't see where portinfo gets used.

>+ union ibv_gid   dgid;

dgid makes more sense in struct pingpong_dest.

> };
>
> struct pingpong_dest {
>@@ -69,6 +73,7 @@
> {
>  struct ibv_qp_attr attr;
>
>+ attr.port_num           = (u8)port;
>  attr.qp_state   = IBV_QPS_RTR;
>  attr.path_mtu   = mtu;
>  attr.dest_qp_num  = dest->qpn;
>@@ -81,6 +86,15 @@
>  attr.ah_attr.src_path_bits = 0;
>  attr.ah_attr.port_num  = (uint8_t) port;

Why isn't this sufficient to specify the port?

>+ if(grh)
>+ {
>+  str2gid(grh, &ctx->dgid);
>+  attr.ah_attr.is_global = 1;
>+  attr.ah_attr.grh.hop_limit = 1;
>+  attr.ah_attr.grh.dgid = ctx->dgid;

Might as well save the dgid to grh.dgid directly, rather than using ctx->dgid as
some temporary space.

>+  attr.ah_attr.grh.sgid_index = 0;
>+ }
>+
>  if (ibv_modify_qp(ctx->qp, &attr,
>      IBV_QP_STATE              |
>      IBV_QP_AV                 |
>@@ -499,7 +513,7 @@
>  while (1) {
>   int c;
>
>-  c = getopt(argc, argv, "h:p:d:i:s:m:r:n:l:e");
>+        c = getopt(argc, argv, "h:p:d:i:s:m:r:n:l:g:e");

usage() must be updated appropriately

>   if (c == -1)
>    break;
>
>@@ -555,6 +569,10 @@
>    servername = _strdup(optarg);
>    break;
>
>+  case 'g':
>+   grh = _strdup(optarg);
>+   break;
>+
>   default:
>    usage(argv[0]);
>    return 1;
>@@ -606,7 +624,7 @@
>  my_dest.psn = rand() & 0xffffff;
>  if (!my_dest.lid) {
>   fprintf(stderr, "Couldn't get local LID\n");
>-  return 1;
>+  //return 1;

This is an unusual change.

- Sean