ib_core: IBoE UD packet packing support
PATCH v10 3/12 ib_core: IBoE UD packet packing support
Add support functions to aid in packing IBoE packets.
--- drivers/infiniband/core/ud_header.c | 128 ++++++++++++++++++++++++++--------- include/rdma/ib_pack.h | 30 +++++++- 2 files changed, 124 insertions(+), 34 deletions(-)
drivers/infiniband/core/ud_header.c +98 -32
@@ -80,6 +80,29 @@ static const struct ib_field lrh_table[] = { .size_bits = 16 } }; +static const struct ib_field eth_table[] = { + {STRUCT_FIELD (eth, dmac_h), + .offset_words =0 , + .offset_bits =0 , + .size_bits =32 }, + {STRUCT_FIELD (eth, dmac_l), + .offset_words =1 , + .offset_bits =0 , + .size_bits =16 }, + {STRUCT_FIELD (eth, smac_h), + .offset_words =1 , + .offset_bits =16 , + .size_bits =16 }, + {STRUCT_FIELD (eth, smac_l), + .offset_words =2 , + .offset_bits =0 , + .size_bits =32 }, + {STRUCT_FIELD (eth, type), + .offset_words =3 , + .offset_bits =0 , + .size_bits =16 } +}; +static const struct ib_field grh_table[] = { {STRUCT_FIELD (grh, ip_version), .offset_words =0 ,
@@ -180,17 +203,15 @@ static const struct ib_field deth_table[] = {/** * ib_ud_header_init - Initialize UD header structure * @payload_bytes:Length of packet payload +* @lrh_present: specify if LRH is present +* @eth_present: specify if Eth header is present * @grh_present:GRH flag (if non-zero, GRH will be included) -* @immediate_present: specify if immediate data should be used +* @immediate_present: specify if immediate data is present * @header:Structure to initialize -* -* ib_ud_header_init() initializes the lrh.link_version, lrh.link_next_header, -* lrh.packet_length, grh.ip_version, grh.payload_length, -* grh.next_header, bth.opcode, bth.pad_count and -* bth.transport_header_version fields of a &struct ib_ud_header given -* the payload length and whether a GRH will be included. */ void ib_ud_header_init (int payload_bytes, +int lrh_present, +int eth_present,int grh_present,int immediate_present,struct ib_ud_header *header)
@@ -199,42 +220,79 @@ void ib_ud_header_init(int payload_bytes,memset (header,0 ,sizeof *header); - header->lrh.link_version =0 ; - header->lrh.link_next_header = - grh_present ?IB_LNH_IBA_GLOBAL :IB_LNH_IBA_LOCAL ; - packet_length = (IB_LRH_BYTES + -IB_BTH_BYTES + -IB_DETH_BYTES + - payload_bytes + -4 +/* ICRC */ -3 ) /4 ;/* round up */ - - header->grh_present = grh_present; +if (lrh_present) { + header->lrh.link_version =0 ; + header->lrh.link_next_header = + grh_present ?IB_LNH_IBA_GLOBAL :IB_LNH_IBA_LOCAL ; + packet_length =IB_LRH_BYTES ; + } + +if (eth_present) + packet_length +=IB_ETH_BYTES ; + packet_length +=IB_BTH_BYTES +IB_DETH_BYTES + payload_bytes + +4 +/* ICRC */ +3 ;/* round up */ + packet_length /=4 ;if (grh_present) { - packet_length +=IB_GRH_BYTES /4 ; - header->grh.ip_version =6 ; - header->grh.payload_length = -cpu_to_be16 ((IB_BTH_BYTES + -IB_DETH_BYTES + - payload_bytes + -4 +/* ICRC */ -3 ) & ~3 );/* round up */ + packet_length +=IB_GRH_BYTES /4 ; + header->grh.ip_version =6 ; + header->grh.payload_length = +cpu_to_be16 ((IB_BTH_BYTES + +IB_DETH_BYTES + + payload_bytes + +4 +/* ICRC */ +3 ) & ~3 );/* round up */ header->grh.next_header =0x1b ; } - header->lrh.packet_length =cpu_to_be16 (packet_length); +if (lrh_present) + header->lrh.packet_length =cpu_to_be16 (packet_length); - header->immediate_present = immediate_present; -if (immediate_present) +if (header->immediate_present) header->bth.opcode =IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE ;else header->bth.opcode =IB_OPCODE_UD_SEND_ONLY ; header->bth.pad_count = (4 - payload_bytes) &3 ; header->bth.transport_header_version =0 ; + + header->lrh_present = lrh_present; + header->eth_present = eth_present; + header->grh_present = grh_present; + header->immediate_present = immediate_present; }EXPORT_SYMBOL (ib_ud_header_init);/** +* ib_lrh_header_pack - Pack LRH header struct into wire format +* @lrh:unpacked LRH header struct +* @buf:Buffer to pack into +* +* ib_lrh_header_pack() packs the LRH header structure @lrh into +* wire format in the buffer @buf. +*/ +int ib_lrh_header_pack (struct ib_unpacked_lrh *lrh,void *buf) +{ +ib_pack (lrh_table,ARRAY_SIZE (lrh_table), lrh, buf); +return 0 ; +} +EXPORT_SYMBOL (ib_lrh_header_pack); + +/** +* ib_lrh_header_unpack - Unpack LRH structure from wire format +* @lrh:unpacked LRH header struct +* @buf:Buffer to pack into +* +* ib_lrh_header_unpack() unpacks the LRH header structure from +* wire format (in buf) into @lrh. +*/ +int ib_lrh_header_unpack (void *buf,struct ib_unpacked_lrh *lrh) +{ +ib_unpack (lrh_table,ARRAY_SIZE (lrh_table), buf, lrh); +return 0 ; +} +EXPORT_SYMBOL (ib_lrh_header_unpack); + +/** * ib_ud_header_pack - Pack UD header struct into wire format * @header:UD header struct * @buf:Buffer to pack into
@@ -247,9 +305,17 @@ int ib_ud_header_pack(struct ib_ud_header *header, {int len =0 ; -ib_pack (lrh_table,ARRAY_SIZE (lrh_table), - &header->lrh, buf); - len +=IB_LRH_BYTES ; +if (header->lrh_present) { +ib_pack (lrh_table,ARRAY_SIZE (lrh_table), + &header->lrh, buf + len); + len +=IB_LRH_BYTES ; + } +if (header->eth_present) { +ib_pack (eth_table,ARRAY_SIZE (eth_table), + &header->eth, buf + len); + len +=IB_ETH_BYTES ; + } +if (header->grh_present) { ib_pack(grh_table, ARRAY_SIZE(grh_table),
include/rdma/ib_pack.h +27 -3
@@ -37,6 +37,7 @@enum {IB_LRH_BYTES =8 , +IB_ETH_BYTES =14 ,IB_GRH_BYTES =40 ,IB_BTH_BYTES =12 ,IB_DETH_BYTES =8
@@ -210,8 +211,29 @@ struct ib_unpacked_deth {__be32 source_qpn; }; +struct ib_unpacked_eth { +u8 dmac_h[4 ]; +u8 dmac_l[2 ]; +u8 smac_h[2 ]; +u8 smac_l[4 ]; +__be16 type; +}; +struct ib_ud_header { +int lrh_present;struct ib_unpacked_lrh lrh; +int eth_present; +struct ib_unpacked_eth eth; +int grh_present; +struct ib_unpacked_grh grh; +struct ib_unpacked_bth bth; +struct ib_unpacked_deth deth; +int immediate_present; +__be32 immediate_data; +}; + +struct eth_ud_header { +struct ib_unpacked_eth eth;int grh_present;struct ib_unpacked_grh grh;struct ib_unpacked_bth bth;
@@ -230,9 +252,11 @@ void ib_unpack(const struct ib_field *desc,void *buf, void *structure); -void ib_ud_header_init (int payload_bytes, -int grh_present, -int immediate_present, +void ib_ud_header_init (int payload_bytes, +int lrh_present, +int eth_present, +int grh_present, +int immediate_present,struct ib_ud_header *header); int ib_ud_header_pack(struct ib_ud_header *header,
Signature · 6 lines
1.7.2.2 -- 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
drivers/infiniband/core/ud_header.c void ib_ud_header_init(int payload_bytes, · line 231
Quoted 9 lines
drivers/infiniband/core/ud_header.c +9
-0+if (lrh_present) { + header->lrh.link_version =0 ; + header->lrh.link_next_header = + grh_present ?IB_LNH_IBA_GLOBAL :IB_LNH_IBA_LOCAL ; + packet_length =IB_LRH_BYTES ; + } + +if (eth_present) + packet_length +=IB_ETH_BYTES ;
How does this code in ib_ud_header_init() work if lrh_present == 0 and eth_present == 1? It seems packet_length is never initialized in that case, or am I missing something?
Anyway I changed the patch in my tree to do
packet_length = IB_ETH_BYTES
if eth_present is set.
I'm assuming lrh_present and eth_present are exclusive; in fact it might be a better interface to ib_ud_header_init() if we passed in a link layer type instead of requiring the caller to do it... that would save from everyone having to do
+ ib_ud_header_init(send_size, !is_eth, is_eth, is_grh, 0, &sqp->ud_header);
as ends up later in the patch series.
This could be a future cleanup if we care.
- R.
--
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 5 lines · Roland Dreier
Quoted 5 lines
drivers/infiniband/core/ud_header.c +5
-0+if (lrh_present) { + header->lrh.link_version =0 ; + header->lrh.link_next_header = + grh_present ?IB_LNH_IBA_GLOBAL :IB_LNH_IBA_LOCAL ; + packet_length =IB_LRH_BYTES ;
Quoted 8 lines
drivers/infiniband/core/ud_header.c void ib_ud_header_init(int payload_bytes, · line 231
Quoted 4 lines
drivers/infiniband/core/ud_header.c +4
-0+ } + +if (eth_present) + packet_length +=IB_ETH_BYTES ;How does this code in
ib_ud_header_init()work iflrh_present== 0 andeth_present== 1? It seemspacket_lengthis never initialized in that case, or am I missing something?
It's a bug but harmless. packet_length is not used in the case of Ethernet link layer so the fact that it is not initialized doesn't cause any harm. Im the case of IB link layer it is initialized.
Quoted 7 lines
Anyway I changed the patch in my tree to do
packet_length=IB_ETH_BYTESif
eth_presentis set.
Maybe just we should just remove any calculations on packet_length for the Ethernet case and simplify the function farther.
Quoted 11 lines
I'm assuming
lrh_presentandeth_presentare exclusive; in fact it might be a better interface toib_ud_header_init()if we passed in a link layer type instead of requiring the caller to do it... that would save from everyone having to do(quoted diff) +1
-0+ ib_ud_header_init(send_size, !is_eth, is_eth, is_grh, 0, &sqp->ud_header);as ends up later in the patch series.
This could be a future cleanup if we care.
Yes, they are mutual exclusive - I agree we can pass in a link layer parameter instead.
--
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 2 lines
Maybe just we should just remove any calculations on
packet_lengthfor the Ethernet case and simplify the function farther.
Makes sense-- I updated my tree to look like:
void ib_ud_header_init(int payload_bytes,
int lrh_present,
int eth_present,
int grh_present,
int immediate_present,
struct ib_ud_header *header)
{
memset(header, 0, sizeof *header);
if (lrh_present) {
u16 packet_length;
header->lrh.link_version = 0;
header->lrh.link_next_header =
grh_present ? IB_LNH_IBA_GLOBAL : IB_LNH_IBA_LOCAL;
packet_length = (IB_LRH_BYTES +
IB_BTH_BYTES +
IB_DETH_BYTES +
grh_present ? IB_GRH_BYTES : 0 +
payload_bytes +
4 + /* ICRC */
3) / 4; /* round up */
header->lrh.packet_length = cpu_to_be16(packet_length);
}
if (grh_present) {
header->grh.ip_version = 6;
header->grh.payload_length =
cpu_to_be16((IB_BTH_BYTES +
IB_DETH_BYTES +
payload_bytes +
4 + /* ICRC */
3) & ~3); /* round up */
header->grh.next_header = 0x1b;
}
if (header->immediate_present)
header->bth.opcode = IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE;
else
header->bth.opcode = IB_OPCODE_UD_SEND_ONLY;
header->bth.pad_count = (4 - payload_bytes) & 3;
header->bth.transport_header_version = 0;
header->lrh_present = lrh_present;
header->eth_present = eth_present;
header->grh_present = grh_present;
header->immediate_present = immediate_present;
}which I think is reasonably clean for now.
--
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 50 lines · Roland Dreier
void ib_ud_header_init(int
payload_bytes,
intlrh_present,
inteth_present,
intgrh_present,
intimmediate_present,
structib_ud_header*header){ memset(header, 0, sizeof *header); if (lrh_present) { u16 packet_length; header->lrh.link_version = 0; header->lrh.link_next_header = grh_present ? IB_LNH_IBA_GLOBAL : IB_LNH_IBA_LOCAL; packet_length = (IB_LRH_BYTES + IB_BTH_BYTES + IB_DETH_BYTES + grh_present ? IB_GRH_BYTES : 0 + payload_bytes + 4 + /* ICRC */ 3) / 4; /* round up */ header->lrh.packet_length = cpu_to_be16(packet_length); } if (grh_present) { header->grh.ip_version = 6; header->grh.payload_length = cpu_to_be16((IB_BTH_BYTES + IB_DETH_BYTES + payload_bytes + 4 + /* ICRC */ 3) & ~3); /* round up */ header->grh.next_header = 0x1b; } if (header->immediate_present) header->bth.opcode = IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE; else header->bth.opcode = IB_OPCODE_UD_SEND_ONLY; header->bth.pad_count = (4 - payload_bytes) & 3; header->bth.transport_header_version = 0; header->lrh_present = lrh_present; header->eth_present = eth_present; header->grh_present = grh_present; header->immediate_present = immediate_present; }
Quoted 1 line
which I think is reasonably clean for now.
Looks good.
--
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
________________________________________
From: [email protected] [[email protected]] On Behalf Of Eli Cohen [[email protected]]
Sent: Thursday, October 14, 2010 2:57 PM
To: Roland Dreier
Cc: RDMA list
Subject: Re: [PATCHv10 03/12] ib_core: IBoE UD packet packing support
Quoted 30 lines · Roland Dreier
void ib_ud_header_init(int
payload_bytes,int lrh_present, int eth_present, int grh_present, int immediate_present, struct ib_ud_header *header) { memset(header, 0, sizeof *header); if (lrh_present) { u16 packet_length; header->lrh.link_version = 0; header->lrh.link_next_header = grh_present ? IB_LNH_IBA_GLOBAL : IB_LNH_IBA_LOCAL; packet_length = (IB_LRH_BYTES + IB_BTH_BYTES + IB_DETH_BYTES + grh_present ? IB_GRH_BYTES : 0 + payload_bytes + 4 + /* ICRC */ 3) / 4; /* round up */ header->lrh.packet_length = cpu_to_be16(packet_length); } if (grh_present) { header->grh.ip_version = 6; header->grh.payload_length = cpu_to_be16((IB_BTH_BYTES +
Quoted 8 lines
IB_DETH_BYTES +
payload_bytes +
4 + /* ICRC */
3) & ~3); /* round up */
header->grh.next_header = 0x1b;
}
if (header->immediate_present)This should be "if (immediate_present)". header->immediate_present is zero due to the memset() above.
Quoted 12 lines
header->bth.opcode = IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE;
else
header->bth.opcode = IB_OPCODE_UD_SEND_ONLY;
header->bth.pad_count = (4 - payload_bytes) & 3;
header->bth.transport_header_version = 0;
header->lrh_present = lrh_present;
header->eth_present = eth_present;
header->grh_present = grh_present;
header->immediate_present = immediate_present;
}Quoted 1 line
which I think is reasonably clean for now.
Looks good.
--
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
--
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
A message that is not in the archive (sent off-list or not yet received): 35AAF1E4A771E142979F27B51793A488873ABE0767-HolNjIBXvBOXx9kJd3VG2h2eb7JE58TQ@public.gmane.org
Quoted 4 lines
Quoted 1 line
if (header->immediate_present)This should be "if (
immediate_present)".header->immediate_presentis zero due to thememset()above.
Thanks, fixed that up.
--
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