Provide support for SubnSet(GUIDInfo)

Hardwire PortInfo.GUIDCap to 1 for both the switch and HCA case,

then simply verify that the incoming SubnSet matches the RO data in block 0.

Signed-off-by Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

---
 ibsim/sim_mad.c |   25 +++++++++++++++++--------
 ibsim/sim_net.c |    2 +-

 2 files changed, 18 insertions(+), 9 deletions(-)

I've put all our patches for ibsim on my github page:

git://github.com/jgunthorpe/ibsim.git

ibsim/sim_mad.c +17 -8

index 83d4e3b..60da7d2 100644
@@ -385,17 +385,26 @@ static int do_guidinfo(Port * port, unsigned op, uint32_t mod, uint8_t * data)
        Node *node = port->node;
        int status = 0;
        uint64_t portguid = node->nodeguid + port->portnum;
+       if (node->type == SWITCH_NODE)
+               portguid = node->nodeguid;
 
-       if (op != IB_MAD_METHOD_GET)    // only get currently supported (non compliant)
-               status = ERR_METHOD_UNSUPPORTED;
-
-       memset(data, 0, IB_SMP_DATA_SIZE);
-       if (mod == 0) {
-               if (node->type == SWITCH_NODE)
-                       mad_encode_field(data, IB_GUID_GUID0_F, &node->nodeguid);
-               else
+       if (op == IB_MAD_METHOD_SET) {
+               /* The PortInfo.GuidCap is hardwired to 1, so the only valid input
+                  to a Set is the port GUID followed by zeros on block 0. */
+               int i = mod == 0?1:0;
+               for (; i != 8; ++i)
+                       if (mad_get_field64(data, i*8, IB_GUID_GUID0_F) != 0)
+                               status = ERR_BAD_PARAM;
+               if (mod != 0 ||
+                   mad_get_field64(data, 0, IB_GUID_GUID0_F) != portguid)
+                       status = ERR_BAD_PARAM;
+       } else if (op == IB_MAD_METHOD_GET) {
+               memset(data, 0, IB_SMP_DATA_SIZE);
+               if (mod == 0)
                        mad_encode_field(data, IB_GUID_GUID0_F, &portguid);
        }
+       else
+               status = ERR_METHOD_UNSUPPORTED;
 
        return status;
  }

ibsim/sim_net.c +1 -2

index 13c3b8c..59fc28e 100644
@@ -104,7 +104,7 @@ static const uint8_t hcaport[] = {
        0x00, 0x00, 0x0F, 0xF9, 0x01, 0x03, 0x03, 0x02,
        0x12, 0x52, 0x00, 0x11, 0x40, 0x40, 0x00, 0x08,
        0x08, 0x04, 0xFF, 0x10, 0x00, 0x00, 0x00, 0x00,
-       0x00, 0x00, 0x20, 0x1F, 0x00, 0x00, 0x00, 0x00,
+       0x00, 0x00, 0x01, 0x1F, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  };
 
--

1.7.1

--
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

Quoted 1 line · Jason Gunthorpe

Hardwire PortInfo.GUIDCap to 1 for both the switch and HCA case,

Why hardwire this to 1 ? This is not what most defaults are.

Quoted 2 lines

then simply verify that the incoming SubnSet matches the RO data in block 0.

Is this just to make the set of guidinfo simple ? Longer term, this should support a GUIDCap > 1 but I suppose that could be a follow on patch to this.

Quoted 6 lines · Hal Rosenstock

Quoted 1 line · Jason Gunthorpe

Hardwire PortInfo.GUIDCap to 1 for both the switch and HCA case,

Why hardwire this to 1 ? This is not what most defaults are.

Quoted 6 lines

Quoted 2 lines

then simply verify that the incoming SubnSet matches the RO data in block 0.

Is this just to make the set of guidinfo simple ? Longer term, this should support a GUIDCap > 1 but I suppose that could be a follow on patch to this.

It is to make the existing ibsim compliant with the spec.

If someone
cares about simulating guidinfo then they can implement a higher port cap, otherwise, setting it to one lets things that those do try to manipulate guid info not blow up.

Jason

Quoted 5 lines

Quoted 3 lines

Is this just to make the set of guidinfo simple ? Longer term, this should support a GUIDCap > 1 but I suppose that could be a follow on patch to this.

It is to make the existing ibsim compliant with the spec.

Yes, supporting set of guidinfo is compliant but I think your treatment of guid 0 in block 0 is overzealous and non compliant.

Quoted 5 lines · Hal Rosenstock

Quoted 5 lines

Quoted 3 lines

Is this just to make the set of guidinfo simple ? Longer term, this should support a GUIDCap > 1 but I suppose that could be a follow on patch to this.

It is to make the existing ibsim compliant with the spec.

Quoted 2 lines

Yes, supporting set of guidinfo is compliant but I think your treatment of guid 0 in block 0 is overzealous and non compliant.

Having sim return an error if an invalid RO field is written is useful from a testing prespective, since nothing should do a set without the correct entry 0 GUID.

A strict reading of the spec would say that no GUIDInfo set is invalid, even if it tries to set non-zero values for entries past the GUIDCap. I don't see how that is useful behavior for ibsim.

Jason

Quoted 7 lines · Jason Gunthorpe

Quoted 5 lines · Hal Rosenstock

Quoted 5 lines

Quoted 3 lines

Is this just to make the set of guidinfo simple ? Longer term, this should support a GUIDCap > 1 but I suppose that could be a follow on patch to this.

It is to make the existing ibsim compliant with the spec.

You wrote that this was for spec compliance...