Hi Hal,
On 08:54 Mon 09 Mar , Hal Rosenstock wrote:
>
> Also, in mad_rpc, status should be based on management class
Please try to not mix two things in one patch.
> @@ -185,10 +186,28 @@ void *mad_rpc(const struct ibmad_port *port, ib_rpc_t *
> rpc, ib_portid_t * dport
>
> mad = umad_get_mad(rcvbuf);
>
> - if ((status = mad_get_field(mad, 0, IB_DRSMP_STATUS_F)) != 0) {
> - ERRS("MAD completed with error status 0x%x; dport (%s)",
> - status, portid2str(dport));
> - return 0;
> + mgmtclass = mad_get_field(mad, 0, IB_MAD_MGMTCLASS_F);
> + if (mgmtclass == 1 || mgmtclass == 0x81) {
> + if (mgmtclass == 1)
> + status = mad_get_field(mad, 0, IB_MAD_STATUS_F);
> + else
> + status = mad_get_field(mad, 0, IB_DRSMP_STATUS_F);
> + if (status != 0) {
> + ERRS("MAD completed with error status 0x%x; dport (%s)",
> + status, portid2str(dport));
> + return 0;
> + }
Basically it is about the same field. Why do we need such flow?
> + } else {
> + if ((status = mad_get_field(mad, 0, IB_MAD_STATUS_F)) != 0) {
} else if (...) {
> + if (status & 2) { /* redirection */
> + ERRS("MAD redirection not supported; dport
> (%s)",
> + portid2str(dport));
> + } else {
> + ERRS("MAD completed with error status 0x%x;
> dport (%s)",
> + status, portid2str(dport));
> + }
You are sending patches which cleans {} around single expressions...
Just wrap ERRS() macro with 'do {..} while (0)' and code will be
cleaner.
> + return 0;
> + }
> }
>
> if (ibdebug) {
> @@ -225,8 +244,13 @@ void *mad_rpc_rmpp(const struct ibmad_port *port,
> ib_rpc_t * rpc, ib_portid_t *
> mad = umad_get_mad(rcvbuf);
>
> if ((status = mad_get_field(mad, 0, IB_MAD_STATUS_F)) != 0) {
> - ERRS("MAD completed with error status 0x%x; dport (%s)",
> - status, portid2str(dport));
> + if (status & 2) { /* redirection */
> + ERRS("MAD redirection not supported; dport (%s)",
> + portid2str(dport));
> + } else {
> + ERRS("MAD completed with error status 0x%x; dport (%s)",
> + status, portid2str(dport));
> + }
Ditto.
Sasha
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
