Skip to contentSkip to conversation

ib_core: IBoE UD packet packing support

PATCH In review Eli Cohen 3 participants 7 messages Aug 26, 2010 – Oct 15, 2010 3 messages not in the archive InfiniBand

lore
Share & tools
lore

Eli Cohen

author
#0 lore raw

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
index 650b501..58b5537 100644
@@ -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
index cbb50f4..67a85f3 100644
@@ -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
#1 lore raw

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]

author
#3 lore raw
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 if lrh_present == 0 and eth_present == 1? It seems packet_length is 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_BYTES

if eth_present is 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_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

(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

#4 lore raw
Quoted 2 lines

Maybe just we should just remove any calculations on packet_length for 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]

author
#6 lore raw
Quoted 50 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     +
				     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

#7 lore raw

________________________________________
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

#9 lore raw
Quoted 4 lines
Quoted 1 line
      if (header->immediate_present)

This should be "if (immediate_present)". header->immediate_present is zero due to the memset() 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