From: Jason Gunthorpe <jgg@ziepe.ca>
To: Leon Romanovsky <leon@kernel.org>
Cc: Doug Ledford <dledford@redhat.com>,
Leon Romanovsky <leonro@mellanox.com>,
RDMA mailing list <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH rdma-next 01/43] RDMA/cm: Add naive SET/GET implementations to hide CM wire format
Date: Fri, 15 Nov 2019 16:45:58 -0400 [thread overview]
Message-ID: <20191115204558.GA22185@ziepe.ca> (raw)
In-Reply-To: <20191027070621.11711-2-leon@kernel.org>
On Sun, Oct 27, 2019 at 09:05:39AM +0200, Leon Romanovsky wrote:
> #define IB_CM_CLASS_VERSION 2 /* IB specification 1.2 */
> +#define _CM_SET(p, offset, mask, value) \
> + ({ \
> + void *field = (u8 *)p + sizeof(struct ib_mad_hdr) + offset; \
> + u8 bytes = \
> + DIV_ROUND_UP(__builtin_popcount(mask), BITS_PER_BYTE); \
> + switch (bytes) { \
> + case 1: { \
> + *(u8 *)field &= ~mask; \
> + *(u8 *)field |= FIELD_PREP(mask, value); \
> + } break; \
> + case 2: { \
> + u16 val = ntohs(*(__be16 *)field) & ~mask; \
> + val |= FIELD_PREP(mask, value); \
> + *(__be16 *)field = htons(val); \
> + } break; \
> + case 3: { \
> + u32 val = ntohl(*(__be32 *)field) & ~(mask << 8); \
> + val |= FIELD_PREP(mask, value) << 8; \
> + *(__be32 *)field = htonl(val); \
This doesn't work for flow label which has a 20 byte field, the <<8 is
just a hack to fix the 24 byte case.
This is also some typo's:
> + #define CM_REQ_LOCAL_EECN_OFFSET 36
> + #define CM_REQ_LOCAL_EECN_MASK GENMASK(24, 0)
Should be 23, 0
> + #define CM_REQ_PRIMARY_PACKET_RATE_OFFSET 91
> + #define CM_REQ_PRIMARY_PACKET_RATE_MASK GENMASK(3, 2)
Packet rate is a 6 bit field, not a 2 bit field
I only looked at REQ. I assume all the others have a similar error
rate.
Overall, I don't like this approach. The macros are messy/buggy and
there isn't a clear mapping of the data in the tables to the C code.
How about this instead:
static inline u32 _cm_get8(const u8 *ptr)
{
return *ptr;
}
static inline void _cm_set8(u8 *ptr, u32 mask, u32 prep_value)
{
*ptr = (*ptr & ~mask) | prep_value;
}
static inline u16 _cm_get16(const __be16 *ptr)
{
return be16_to_cpu(*ptr);
}
static inline void _cm_set16(__be16 *ptr, u16 mask, u16 prep_value)
{
*ptr = cpu_to_be16((be16_to_cpu(*ptr) & ~mask) | prep_value);
}
static inline u32 _cm_get32(const __be32 *ptr)
{
return be32_to_cpu(*ptr);
}
static inline void _cm_set32(__be32 *ptr, u32 mask, u32 prep_value)
{
*ptr = cpu_to_be32((be32_to_cpu(*ptr) & ~mask) | prep_value);
}
static inline u64 _cm_get64(const __be64 *ptr)
{
/*
* The mads are constructed so that 32 bit and smaller are naturally
* aligned, everything larger has a max alignment of 4 bytes.
*/
return be64_to_cpu(get_unaligned(ptr));
}
static inline void _cm_set64(__be64 *ptr, u64 mask, u64 prep_value)
{
put_unaligned(cpu_to_be64((_cm_get64(ptr) & ~mask) | prep_value), ptr);
}
#define _CM_SET(field_struct, field_offset, field_mask, mask_width, ptr, \
value) \
({ \
field_struct *_ptr = ptr; \
_cm_set##mask_width((void *)_ptr + (field_offset), field_mask, \
FIELD_PREP(field_mask, value)); \
})
#define CM_SET(field, ptr, value) _CM_SET(field, ptr, value)
#define _CM_SET_GID(field, ptr, val) \
({ \
field_struct *_ptr = ptr; \
const union ib_gid *_val = val; \
memcpy((void *)_ptr + field_offset, _val, sizeof(*_out)); \
})
#define CM_SET_GID(field, ptr, val) _CM_SET_GID(field, ptr, val)
#define _CM_GET(field_struct, field_offset, field_mask, mask_width, ptr) \
({ \
const field_struct *_ptr = ptr; \
(u##mask_width) FIELD_GET( \
field_mask, _cm_get##mask_width((const void *)_ptr + \
(field_offset))); \
})
#define CM_GET(field, ptr) _CM_GET(field, ptr)
#define _CM_GET_GID(field_struct, field_offset, ptr, out) \
({ \
const field_struct *_ptr = ptr; \
union ib_gid *_out = out; \
memcpy(_out, (void *)_ptr + field_offset, sizeof(*_out)); \
})
#define CM_GET_GID(field, ptr, out) _CM_GET_GID(field, ptr, out)
/*
* The generated list becomes the parameters to the macros, the order is:
* - struct this applies to
* - starting offset of the max
* - GENMASK or GENMASK_ULL in CPU order
* - The width of data the mask operations should work on, in bits
*/
/*
* Extraction using a tabular description like table 106. bit_offset is from
* the Byte[Bit] notation.
*/
#define IBA_FIELD_BLOC(field_struct, byte_offset, bit_offset, num_bits) \
field_struct, byte_offset, \
GENMASK(7 - (bit_offset), 7 - (bit_offset) - (num_bits - 1)), \
8
#define IBA_FIELD8_LOC(field_struct, byte_offset, num_bits) \
IBA_FIELD_BLOC(field_struct, byte_offset, 0, num_bits)
#define IBA_FIELD16_LOC(field_struct, byte_offset, num_bits) \
field_struct, (byte_offset)&0xFFFE, \
GENMASK(15 - (((byte_offset) % 2) * 8), \
15 - (((byte_offset) % 2) * 8) - (num_bits - 1)), \
16
#define IBA_FIELD32_LOC(field_struct, byte_offset, num_bits) \
field_struct, (byte_offset)&0xFFFC, \
GENMASK(31 - (((byte_offset) % 4) * 8), \
31 - (((byte_offset) % 4) * 8) - (num_bits - 1)), \
32
#define IBA_FIELD64_LOC(field_struct, byte_offset, num_bits) \
field_struct, (byte_offset)&0xFFF8, \
GENMASK_ULL(63 - (((byte_offset) % 8) * 8), \
63 - (((byte_offset) % 8) * 8) - (num_bits - 1)), \
64
#define IBA_FIELD_GID_LOC(field_struct, byte_offset) field_struct, byte_offset
/* From Table 106 */
#define CM_REQ_LOCAL_COMM_ID IBA_FIELD32_LOC(struct cm_req_msg, 0, 32)
#define CM_REQ_SERVICE_ID IBA_FIELD64_LOC(struct cm_req_msg, 8, 64)
#define CM_REQ_LOCAL_CA_GUID IBA_FIELD64_LOC(struct cm_req_msg, 16, 64)
#define CM_REQ_LOCAL_Q_KEY IBA_FIELD32_LOC(struct cm_req_msg, 28, 32)
#define CM_REQ_LOCAL_QPN IBA_FIELD32_LOC(struct cm_req_msg, 32, 24)
#define CM_REQ_RESPONDED_RESOURCES IBA_FIELD8_LOC(struct cm_req_msg, 35, 8)
#define CM_REQ_LOCAL_EECN IBA_FIELD32_LOC(struct cm_req_msg, 36, 24)
#define CM_REQ_INITIATOR_DEPTH IBA_FIELD8_LOC(struct cm_req_msg, 39, 8)
#define CM_REQ_REMOTE_EECN IBA_FIELD32_LOC(struct cm_req_msg, 40, 24)
#define CM_REQ_REMOTE_CM_RESPONSE_TIMEOUT IBA_FIELD8_LOC(struct cm_req_msg, 43, 5)
#define CM_REQ_TRANSPORT_SERVICE_TYPE IBA_FIELD_BLOC(struct cm_req_msg, 43, 5, 2)
#define CM_REQ_END_TO_END_FLOW_CONTROL IBA_FIELD_BLOC(struct cm_req_msg, 43, 7, 1)
#define CM_REQ_STARTING_PSN IBA_FIELD32_LOC(struct cm_req_msg, 44, 24)
#define CM_REQ_LOCAL_CM_RESPONSE_TIMEOUT IBA_FIELD8_LOC(struct cm_req_msg, 47, 5)
#define CM_REQ_RETRY_COUNT IBA_FIELD_BLOC(struct cm_req_msg, 47, 5, 3)
#define CM_REQ_PARTITION_KEY IBA_FIELD16_LOC(struct cm_req_msg, 48, 16)
#define CM_REQ_PATH_PACKET_PAYLOAD_MTU IBA_FIELD8_LOC(struct cm_req_msg, 50, 4)
#define CM_REQ_RDC_EXISTS IBA_FIELD_BLOC(struct cm_req_msg, 50, 4, 1)
#define CM_REQ_RNR_RETRY_COUNT IBA_FIELD_BLOC(struct cm_req_msg, 50, 5, 3)
#define CM_REQ_MAX_CM_RETRIES IBA_FIELD8_LOC(struct cm_req_msg, 51, 4)
#define CM_REQ_SRQ IBA_FIELD_BLOC(struct cm_req_msg, 51, 4, 1)
#define CM_REQ_EXTENDED_TRANSPORT_TYPE IBA_FIELD_BLOC(struct cm_req_msg, 51, 5, 3)
#define CM_REQ_PRIMARY_LOCAL_PORT_LID IBA_FIELD16_LOC(struct cm_req_msg, 52, 16)
#define CM_REQ_PRIMARY_REMOTE_PORT_LID IBA_FIELD16_LOC(struct cm_req_msg, 54, 16)
#define CM_REQ_PRIMARY_LOCAL_PORT_GID IBA_FIELD_GID_LOC(struct cm_req_msg, 56)
#define CM_REQ_PRIMARY_REMOTE_PORT_GID IBA_FIELD_GID_LOC(struct cm_req_msg, 72)
#define CM_REQ_PRIMARY_FLOW_LABEL IBA_FIELD32_LOC(struct cm_req_msg, 88, 20)
#define CM_REQ_PRIMARY_PACKET_RATE IBA_FIELD_BLOC(struct cm_req_msg, 91, 2, 6)
#define CM_REQ_PRIMARY_TRAFFIC_CLASS IBA_FIELD8_LOC(struct cm_req_msg, 92, 8)
#define CM_REQ_PRIMARY_HOP_LIMIT IBA_FIELD8_LOC(struct cm_req_msg, 93, 8)
#define CM_REQ_PRIMARY_SL IBA_FIELD8_LOC(struct cm_req_msg, 94, 4)
#define CM_REQ_PRIMARY_SUBNET_LOCAL IBA_FIELD_BLOC(struct cm_req_msg, 94, 4, 1)
#define CM_REQ_PRIMARY_LOCAL_ACK_TIMEOUT IBA_FIELD8_LOC(struct cm_req_msg, 95, 5)
#define CM_REQ_ALTERNATE_LOCAL_PORT_LID IBA_FIELD16_LOC(struct cm_req_msg, 96, 16)
#define CM_REQ_ALTERNATE_REMOTE_PORT_LID IBA_FIELD16_LOC(struct cm_req_msg, 98, 16)
#define CM_REQ_ALTERNATE_LOCAL_PORT_GID IBA_FIELD_GID_LOC(struct cm_req_msg, 100)
#define CM_REQ_ALTERNATE_REMOTE_PORT_GID IBA_FIELD_GID_LOC(struct cm_req_msg, 116)
#define CM_REQ_ALTERNATE_FLOW_LABEL IBA_FIELD32_LOC(struct cm_req_msg, 132, 20)
#define CM_REQ_ALTERNATE_PACKET_RATE IBA_FIELD_BLOC(struct cm_req_msg, 135, 2, 6)
#define CM_REQ_ALTERNATE_TRAFFIC_CLASS IBA_FIELD8_LOC(struct cm_req_msg, 136, 8)
#define CM_REQ_ALTERNATE_HOP_LIMIT IBA_FIELD8_LOC(struct cm_req_msg, 137, 8)
#define CM_REQ_ALTERNATE_SL IBA_FIELD8_LOC(struct cm_req_msg, 138, 4)
#define CM_REQ_ALTERNATE_SUBNET_LOCAL IBA_FIELD_BLOC(struct cm_req_msg, 138, 4, 1)
#define CM_REQ_ALTERNATE_LOCAL_ACK_TIMEOUT IBA_FIELD8_LOC(struct cm_req_msg, 139, 5)
#define CM_REQ_SAP_SUPPORTED IBA_FIELD_BLOC(struct cm_req_msg, 139, 5, 1)
next prev parent reply other threads:[~2019-11-15 20:46 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-27 7:05 [PATCH rdma-next 00/43] Convert CM to bitmaps Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 01/43] RDMA/cm: Add naive SET/GET implementations to hide CM wire format Leon Romanovsky
2019-11-15 20:45 ` Jason Gunthorpe [this message]
2019-11-18 13:04 ` Leon Romanovsky
2019-11-18 13:43 ` Jason Gunthorpe
2019-10-27 7:05 ` [PATCH rdma-next 02/43] RDMA/cm: Request For Communication (REQ) message definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 03/43] RDMA/cm: Message Receipt Acknowledgment (MRA) " Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 04/43] RDMA/cm: Reject (REJ) " Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 05/43] RDMA/cm: Reply To Request for communication (REP) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 06/43] RDMA/cm: Ready To Use (RTU) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 07/43] RDMA/cm: Request For Communication Release (DREQ) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 08/43] RDMA/cm: Reply To Request For Communication Release (DREP) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 09/43] RDMA/cm: Load Alternate Path (LAP) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 10/43] RDMA/cm: Alternate Path Response (APR) message definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 11/43] RDMA/cm: Service ID Resolution Request (SIDR_REQ) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 12/43] RDMA/cm: Service ID Resolution Response (SIDR_REP) definitions Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 13/43] RDMA/cm: Convert QPN and EECN to be u32 variables Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 14/43] RDMA/cm: Convert REQ responded resources to the new scheme Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 15/43] RDMA/cm: Convert REQ initiator depth " Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 16/43] RDMA/cm: Convert REQ remote response timeout Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 17/43] RDMA/cm: Simplify QP type to wire protocol translation Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 18/43] RDMA/cm: Convert REQ flow control Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 19/43] RDMA/cm: Convert starting PSN to be u32 variable Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 20/43] RDMA/cm: Update REQ local response timeout Leon Romanovsky
2019-10-27 7:05 ` [PATCH rdma-next 21/43] RDMA/cm: Convert REQ retry count to use new scheme Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 22/43] RDMA/cm: Update REQ path MTU field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 23/43] RDMA/cm: Convert REQ RNR retry timeout counter Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 24/43] RDMA/cm: Convert REQ MAX CM retries Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 25/43] RDMA/cm: Convert REQ SRQ field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 26/43] RDMA/cm: Convert REQ flow label field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 27/43] RDMA/cm: Convert REQ packet rate Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 28/43] RDMA/cm: Convert REQ SL fields Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 29/43] RDMA/cm: Convert REQ subnet local fields Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 30/43] RDMA/cm: Convert REQ local ack timeout Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 31/43] RDMA/cm: Convert MRA MRAed field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 32/43] RDMA/cm: Convert MRA service timeout Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 33/43] RDMA/cm: Update REJ struct to use new scheme Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 34/43] RDMA/cm: Convert REP target ack delay field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 35/43] RDMA/cm: Convert REP failover accepted field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 36/43] RDMA/cm: Convert REP flow control field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 37/43] RDMA/cm: Convert REP RNR retry count field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 38/43] RDMA/cm: Convert REP SRQ field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 39/43] RDMA/cm: Delete unused CM LAP functions Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 40/43] RDMA/cm: Convert LAP flow label field Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 41/43] RDMA/cm: Convert LAP fields Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 42/43] RDMA/cm: Delete unused CM ARP functions Leon Romanovsky
2019-10-27 7:06 ` [PATCH rdma-next 43/43] RDMA/cm: Convert SIDR_REP to new scheme Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191115204558.GA22185@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=dledford@redhat.com \
--cc=leon@kernel.org \
--cc=leonro@mellanox.com \
--cc=linux-rdma@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.