[PATCH 0/3] Receive Side Coalescing for macb driver

[PATCH 0/3] Receive Side Coalescing for macb driver

[Rafal Ozieblo]

This patch series adds support for receive side coalescing
for Cadence GEM driver. Receive segmentation coalescing
is a mechanism to reduce CPU overhead. This is done by
coalescing received TCP message segments together into
a single large message. This means that when the message
is complete the CPU only has to process the single header
and act upon the one data payload.

Rafal Ozieblo (3):
  net: macb: Add support for rsc capable hardware
  net: macb: Add support for header data spliting
  net: macb: Receive Side Coalescing (RSC) feature added.

 drivers/net/ethernet/cadence/macb.h      |  21 +++
 drivers/net/ethernet/cadence/macb_main.c | 227 ++++++++++++++++++++++++++-----
 2 files changed, 212 insertions(+), 36 deletions(-)

-- 
2.4.5

[PATCH 1/3] net: macb: Add support for rsc capable hardware

[Rafal Ozieblo]

When the pbuf_rsc has been enabled in hardware
the receive buffer offset for incoming packets
cannot be changed in the network configuration register
(even when rsc is not use at all).

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 22 ++++++++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8665982..33c9a48 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -477,6 +477,8 @@
 /* Bitfields in DCFG6. */
 #define GEM_PBUF_LSO_OFFSET			27
 #define GEM_PBUF_LSO_SIZE			1
+#define GEM_PBUF_RSC_OFFSET			26
+#define GEM_PBUF_RSC_SIZE			1
 #define GEM_DAW64_OFFSET			23
 #define GEM_DAW64_SIZE				1
 
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b4c9268..43201a8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -930,8 +930,9 @@ static void gem_rx_refill(struct macb_queue *queue)
 			macb_set_addr(bp, desc, paddr);
 			desc->ctrl = 0;
 
-			/* properly align Ethernet header */
-			skb_reserve(skb, NET_IP_ALIGN);
+			if (!(bp->dev->hw_features & NETIF_F_LRO))
+				/* properly align Ethernet header */
+				skb_reserve(skb, NET_IP_ALIGN);
 		} else {
 			desc->addr &= ~MACB_BIT(RX_USED);
 			desc->ctrl = 0;
@@ -2110,7 +2111,13 @@ static void macb_init_hw(struct macb *bp)
 	config = macb_mdc_clk_div(bp);
 	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
 		config |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
-	config |= MACB_BF(RBOF, NET_IP_ALIGN);	/* Make eth data aligned */
+	/* When the pbuf_rsc has been enabled in hardware the receive buffer
+	 * offset cannot be changed in the network configuration register.
+	 */
+	if (!(bp->dev->hw_features &  NETIF_F_LRO))
+		/* Make eth data aligned */
+		config |= MACB_BF(RBOF, NET_IP_ALIGN);
+
 	config |= MACB_BIT(PAE);		/* PAuse Enable */
 	config |= MACB_BIT(DRFCS);		/* Discard Rx FCS */
 	if (bp->caps & MACB_CAPS_JUMBO)
@@ -2281,7 +2288,7 @@ static void macb_set_rx_mode(struct net_device *dev)
 static int macb_open(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
-	size_t bufsz = dev->mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN;
+	size_t bufsz = dev->mtu + ETH_HLEN + ETH_FCS_LEN;
 	struct macb_queue *queue;
 	unsigned int q;
 	int err;
@@ -2295,6 +2302,9 @@ static int macb_open(struct net_device *dev)
 	if (!dev->phydev)
 		return -EAGAIN;
 
+	if (!(bp->dev->hw_features & NETIF_F_LRO))
+		bufsz += NET_IP_ALIGN;
+
 	/* RX buffers initialization */
 	macb_init_rx_buffer_size(bp, bufsz);
 
@@ -3365,6 +3375,10 @@ static int macb_init(struct platform_device *pdev)
 	if (GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
 		dev->hw_features |= MACB_NETIF_LSO;
 
+	/* Check RSC capability */
+	if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
+		dev->hw_features |= NETIF_F_LRO;
+
 	/* Checksum offload is only available on gem with packet buffer */
 	if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE))
 		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
-- 
2.4.5

[PATCH 2/3] net: macb: Add support for header data spliting

[Rafal Ozieblo]

This patch adds support for frames splited between
many rx buffers. Header data spliting can be used
but also buffers shorter than max frame length.
The only limitation is that frame header can't
be splited.

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      |  13 +++
 drivers/net/ethernet/cadence/macb_main.c | 137 +++++++++++++++++++++++--------
 2 files changed, 118 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 33c9a48..a2cb805 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -295,6 +295,8 @@
 /* Bitfields in DMACFG. */
 #define GEM_FBLDO_OFFSET	0 /* fixed burst length for DMA */
 #define GEM_FBLDO_SIZE		5
+#define GEM_HDRS_OFFSET		5 /* Header Data Splitting */
+#define GEM_HDRS_SIZE		1
 #define GEM_ENDIA_DESC_OFFSET	6 /* endian swap mode for management descriptor access */
 #define GEM_ENDIA_DESC_SIZE	1
 #define GEM_ENDIA_PKT_OFFSET	7 /* endian swap mode for packet data access */
@@ -755,8 +757,12 @@ struct gem_tx_ts {
 #define MACB_RX_SOF_SIZE			1
 #define MACB_RX_EOF_OFFSET			15
 #define MACB_RX_EOF_SIZE			1
+#define MACB_RX_HDR_OFFSET			16
+#define MACB_RX_HDR_SIZE			1
 #define MACB_RX_CFI_OFFSET			16
 #define MACB_RX_CFI_SIZE			1
+#define MACB_RX_EOH_OFFSET			17
+#define MACB_RX_EOH_SIZE			1
 #define MACB_RX_VLAN_PRI_OFFSET			17
 #define MACB_RX_VLAN_PRI_SIZE			3
 #define MACB_RX_PRI_TAG_OFFSET			20
@@ -1086,6 +1092,11 @@ struct tsu_incr {
 	u32 ns;
 };
 
+struct rx_frag_list {
+	struct sk_buff		*skb_head;
+	struct sk_buff		*skb_tail;
+};
+
 struct macb_queue {
 	struct macb		*bp;
 	int			irq;
@@ -1121,6 +1132,8 @@ struct macb_queue {
 	unsigned int		tx_ts_head, tx_ts_tail;
 	struct gem_tx_ts	tx_timestamps[PTP_TS_BUFFER_SIZE];
 #endif
+	struct rx_frag_list	rx_frag;
+	u32			rx_frag_len;
 };
 
 struct ethtool_rx_fs_item {
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 43201a8..27c406c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -967,6 +967,13 @@ static void discard_partial_frame(struct macb_queue *queue, unsigned int begin,
 	 */
 }
 
+void gem_reset_rx_state(struct macb_queue *queue)
+{
+	queue->rx_frag.skb_head = NULL;
+	queue->rx_frag.skb_tail = NULL;
+	queue->rx_frag_len = 0;
+}
+
 static int gem_rx(struct macb_queue *queue, int budget)
 {
 	struct macb *bp = queue->bp;
@@ -977,6 +984,9 @@ static int gem_rx(struct macb_queue *queue, int budget)
 	int			count = 0;
 
 	while (count < budget) {
+		struct sk_buff *skb_head, *skb_tail;
+		bool eoh = false, header = false;
+		bool sof, eof;
 		u32 ctrl;
 		dma_addr_t addr;
 		bool rxused;
@@ -995,57 +1005,118 @@ static int gem_rx(struct macb_queue *queue, int budget)
 			break;
 
 		queue->rx_tail++;
-		count++;
-
-		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
+		skb = queue->rx_skbuff[entry];
+		if (unlikely(!skb)) {
 			netdev_err(bp->dev,
-				   "not whole frame pointed by descriptor\n");
+				   "inconsistent Rx descriptor chain\n");
 			bp->dev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
 		}
-		skb = queue->rx_skbuff[entry];
-		if (unlikely(!skb)) {
+		skb_head = queue->rx_frag.skb_head;
+		skb_tail = queue->rx_frag.skb_tail;
+		sof = !!(ctrl & MACB_BIT(RX_SOF));
+		eof = !!(ctrl & MACB_BIT(RX_EOF));
+		if (GEM_BFEXT(HDRS, gem_readl(bp, DMACFG))) {
+			eoh = !!(ctrl & MACB_BIT(RX_EOH));
+			if (!eof)
+				header = !!(ctrl & MACB_BIT(RX_HDR));
+		}
+
+		queue->rx_skbuff[entry] = NULL;
+		/* Discard if out-of-sequence or header split across buffers */
+		if ((!skb_head /* first frame buffer */
+		&& (!sof /* without start of frame */
+		|| (header && !eoh))) /* or without whole header */
+		|| (skb_head && sof)) { /* or new start before EOF */
+			struct sk_buff *tmp_skb;
+
 			netdev_err(bp->dev,
-				   "inconsistent Rx descriptor chain\n");
+				   "Incomplete frame received! (skb_head=%p sof=%u hdr=%u eoh=%u)\n",
+				   skb_head, (u32)sof, (u32)header, (u32)eoh);
+			dev_kfree_skb(skb);
+			if (skb_head) {
+				skb = skb_shinfo(skb_head)->frag_list;
+				dev_kfree_skb(skb_head);
+				while (skb) {
+					tmp_skb = skb;
+					skb = skb->next;
+					dev_kfree_skb(tmp_skb);
+				}
+			}
 			bp->dev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
+			gem_reset_rx_state(queue);
 			break;
 		}
+
 		/* now everything is ready for receiving packet */
-		queue->rx_skbuff[entry] = NULL;
 		len = ctrl & bp->rx_frm_len_mask;
 
+		/* Buffer lengths in the descriptor:
+		 * eoh: len = header size,
+		 * eof: len = frame size (including header),
+		 * else: len = 0, length equals bp->rx_buffer_size
+		 */
+		if (!len)
+			len = bp->rx_buffer_size;
+		else
+			/* If EOF or EOH reduce the size of the packet
+			 * by already received bytes
+			 */
+			len -= queue->rx_frag_len;
+
 		netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
 
+		gem_ptp_do_rxstamp(bp, skb, desc);
+
 		skb_put(skb, len);
 		dma_unmap_single(&bp->pdev->dev, addr,
 				 bp->rx_buffer_size, DMA_FROM_DEVICE);
 
-		skb->protocol = eth_type_trans(skb, bp->dev);
-		skb_checksum_none_assert(skb);
-		if (bp->dev->features & NETIF_F_RXCSUM &&
-		    !(bp->dev->flags & IFF_PROMISC) &&
-		    GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
-
-		bp->dev->stats.rx_packets++;
-		queue->stats.rx_packets++;
-		bp->dev->stats.rx_bytes += skb->len;
-		queue->stats.rx_bytes += skb->len;
-
-		gem_ptp_do_rxstamp(bp, skb, desc);
-
-#if defined(DEBUG) && defined(VERBOSE_DEBUG)
-		netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
-			    skb->len, skb->csum);
-		print_hex_dump(KERN_DEBUG, " mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
-			       skb_mac_header(skb), 16, true);
-		print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_ADDRESS, 16, 1,
-			       skb->data, 32, true);
-#endif
-
-		netif_receive_skb(skb);
+		if (!skb_head) {
+			/* first buffer in frame */
+			skb->protocol = eth_type_trans(skb, bp->dev);
+			skb_checksum_none_assert(skb);
+			if (bp->dev->features & NETIF_F_RXCSUM &&
+			    !(bp->dev->flags & IFF_PROMISC) &&
+			    GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
+				skb->ip_summed = CHECKSUM_UNNECESSARY;
+			queue->rx_frag.skb_head = skb;
+			queue->rx_frag.skb_tail = skb;
+			skb_head = skb;
+		} else {
+			/* not first buffer in frame */
+			if (!skb_shinfo(skb_head)->frag_list)
+				skb_shinfo(skb_head)->frag_list = skb;
+			else
+				skb_tail->next = skb;
+			queue->rx_frag.skb_tail = skb;
+			skb_head->len += len;
+			skb_head->data_len += len;
+			skb_head->truesize += len;
+		}
+		if (eof) {
+			bp->dev->stats.rx_packets++;
+			queue->stats.rx_packets++;
+			bp->dev->stats.rx_bytes += skb->len;
+			queue->stats.rx_bytes += skb->len;
+
+	#if defined(DEBUG) && defined(VERBOSE_DEBUG)
+			netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
+				    skb->len, skb->csum);
+			print_hex_dump(KERN_DEBUG, " mac: ", DUMP_PREFIX_ADDRESS, 16, 1,
+				       skb_mac_header(skb), 16, true);
+			print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_ADDRESS, 16, 1,
+				       skb->data, 32, true);
+	#endif
+
+			netif_receive_skb(skb_head);
+			gem_reset_rx_state(queue);
+			count++;
+		} else {
+			queue->rx_frag_len += len;
+		}
 	}
 
 	gem_rx_refill(queue);
@@ -1905,6 +1976,8 @@ static int macb_alloc_consistent(struct macb *bp)
 		netdev_dbg(bp->dev,
 			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
 			   size, (unsigned long)queue->rx_ring_dma, queue->rx_ring);
+
+		gem_reset_rx_state(queue);
 	}
 	if (bp->macbgem_ops.mog_alloc_rx_buffers(bp))
 		goto out_err;
-- 
2.4.5

[PATCH 3/3] net: macb: Receive Side Coalescing (RSC) feature added.

[Rafal Ozieblo]

This is basically the same as Large Receive Offload (LRO)
in Linux framework.

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      |  6 +++
 drivers/net/ethernet/cadence/macb_main.c | 70 +++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index a2cb805..9ebdde7 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -83,6 +83,7 @@
 #define GEM_USRIO		0x000c /* User IO */
 #define GEM_DMACFG		0x0010 /* DMA Configuration */
 #define GEM_JML			0x0048 /* Jumbo Max Length */
+#define GEM_RSC			0x0058 /* RSC Control */
 #define GEM_HRB			0x0080 /* Hash Bottom */
 #define GEM_HRT			0x0084 /* Hash Top */
 #define GEM_SA1B		0x0088 /* Specific1 Bottom */
@@ -318,6 +319,11 @@
 #define GEM_ADDR64_OFFSET	30 /* Address bus width - 64b or 32b */
 #define GEM_ADDR64_SIZE		1
 
+/* Bitfields in RSC control */
+#define GEM_RSCCTRL_OFFSET	1 /* RSC control */
+#define GEM_RSCCTRL_SIZE	15
+#define GEM_CLRMSK_OFFSET	16 /* RSC clear mask */
+#define GEM_CLRMSK_SIZE		1
 
 /* Bitfields in NSR */
 #define MACB_NSR_LINK_OFFSET	0 /* pcs_link_state */
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 27c406c..92bdcf1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2377,6 +2377,8 @@ static int macb_open(struct net_device *dev)
 
 	if (!(bp->dev->hw_features & NETIF_F_LRO))
 		bufsz += NET_IP_ALIGN;
+	else
+		bufsz = 0xFF * 64; // For RSC Buffer Sizes must be set to 16K.
 
 	/* RX buffers initialization */
 	macb_init_rx_buffer_size(bp, bufsz);
@@ -2801,6 +2803,62 @@ static int macb_get_ts_info(struct net_device *netdev,
 	return ethtool_op_get_ts_info(netdev, info);
 }
 
+static void gem_enable_hdr_data_split(struct macb *bp, bool enable)
+{
+	u32 dmacfg;
+
+	dmacfg = gem_readl(bp, DMACFG);
+	if (enable)
+		dmacfg |= GEM_BIT(HDRS);
+	else
+		dmacfg &= ~GEM_BIT(HDRS);
+	gem_writel(bp, DMACFG, dmacfg);
+}
+
+static void gem_update_rsc_state(struct macb *bp, netdev_features_t feature)
+{
+	u32 rsc_control, rsc_control_new, queue, rsc;
+	bool enable, jumbo, any_enabled = false;
+	struct ethtool_rx_fs_item *item;
+	unsigned long flags;
+	u32 ncfgr;
+
+	enable = (!!(feature & NETIF_F_NTUPLE) && !!(feature & NETIF_F_LRO));
+	rsc = gem_readl(bp, RSC);
+	rsc_control = GEM_BFEXT(RSCCTRL, rsc);
+	rsc_control_new = 0;
+	if (enable) {
+		list_for_each_entry(item, &bp->rx_fs_list.list, list) {
+			queue = item->fs.ring_cookie;
+			rsc_control_new |= (1 << (queue - 1));
+			any_enabled = true;
+			netdev_dbg(bp->dev, "RSC %sabled for queue %u\n",
+				   enable ? "en" : "dis", queue);
+		}
+	}
+	if (rsc_control_new != rsc_control) {
+		rsc = GEM_BFINS(RSCCTRL, rsc_control_new, rsc);
+		gem_writel(bp, RSC, rsc);
+	}
+	if (bp->caps & MACB_CAPS_JUMBO) {
+		/* Don't enable jumbo mode for RSC:
+		 * disable unless not RSC and large MTU
+		 */
+		ncfgr = gem_readl(bp, NCFGR);
+		enable = !any_enabled;
+		jumbo = !!MACB_BFEXT(JFRAME, ncfgr);
+		/* and don't touch if already in the state we want */
+		if ((jumbo && !enable) || (!jumbo && enable)) {
+			ncfgr = MACB_BFINS(JFRAME, enable, ncfgr);
+			spin_lock_irqsave(&bp->lock, flags);
+			gem_writel(bp, NCFGR, ncfgr);
+			spin_unlock_irqrestore(&bp->lock, flags);
+		}
+	}
+	/* Need to enable header-data splitting also */
+	gem_enable_hdr_data_split(bp, any_enabled);
+}
+
 static void gem_enable_flow_filters(struct macb *bp, bool enable)
 {
 	struct ethtool_rx_fs_item *item;
@@ -2969,6 +3027,8 @@ static int gem_add_flow_filter(struct net_device *netdev,
 	if (netdev->features & NETIF_F_NTUPLE)
 		gem_enable_flow_filters(bp, 1);
 
+	/* enable RSC if LRO & NTUPLE on */
+	gem_update_rsc_state(bp, netdev->features);
 	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	return 0;
 
@@ -3009,6 +3069,7 @@ static int gem_del_flow_filter(struct net_device *netdev,
 			return 0;
 		}
 	}
+	gem_update_rsc_state(bp, netdev->features);
 
 	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	return -EINVAL;
@@ -3191,7 +3252,12 @@ static int macb_set_features(struct net_device *netdev,
 		bool turn_on = features & NETIF_F_NTUPLE;
 
 		gem_enable_flow_filters(bp, turn_on);
+		gem_update_rsc_state(bp, features);
 	}
+
+	/* LRO (Large Receive Offload) aka RSC (Receive Side Coalescing) */
+	if ((changed & NETIF_F_LRO) && macb_is_gem(bp))
+		gem_update_rsc_state(bp, features);
 	return 0;
 }
 
@@ -3449,8 +3515,10 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features |= MACB_NETIF_LSO;
 
 	/* Check RSC capability */
-	if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
+	if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6))) {
 		dev->hw_features |= NETIF_F_LRO;
+		gem_writel(bp, RSC, GEM_BIT(CLRMSK));
+	}
 
 	/* Checksum offload is only available on gem with packet buffer */
 	if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE))
-- 
2.4.5

Re: [PATCH 0/3] Receive Side Coalescing for macb driver

[David Miller]

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Sat, 14 Apr 2018 21:53:07 +0100

> This patch series adds support for receive side coalescing
> for Cadence GEM driver. Receive segmentation coalescing
> is a mechanism to reduce CPU overhead. This is done by
> coalescing received TCP message segments together into
> a single large message. This means that when the message
> is complete the CPU only has to process the single header
> and act upon the one data payload.

You're going to have to think more deeply about enabling this
feature.

If you can't adjust the receive buffer offset, then the various
packet header fields will be unaligned.

On certain architectures this will result in unaligned traps
all over the networking stack as the packet is being processed.

So enabling this by default will hurt performance on such
systems a lot.

The whole "skb_reserve(skb, NET_IP_ALIGN)" is not just for fun,
it is absolutely essential.

RE: [PATCH 0/3] Receive Side Coalescing for macb driver

[Rafal Ozieblo]

From: David Miller [mailto:davem@davemloft.net] 
Sent: 16 kwietnia 2018 17:09

> From: Rafal Ozieblo <rafalo@cadence.com>
> Date: Sat, 14 Apr 2018 21:53:07 +0100
>
>> This patch series adds support for receive side coalescing for Cadence 
>> GEM driver. Receive segmentation coalescing is a mechanism to reduce 
>> CPU overhead. This is done by coalescing received TCP message segments 
>> together into a single large message. This means that when the message
>> is complete the CPU only has to process the single header and act upon 
>> the one data payload.
>
> You're going to have to think more deeply about enabling this feature.
>
> If you can't adjust the receive buffer offset, then the various packet header fields will be unaligned.
>
> On certain architectures this will result in unaligned traps all over the networking stack as the packet is being processed.
>
> So enabling this by default will hurt performance on such systems a lot.
>
> The whole "skb_reserve(skb, NET_IP_ALIGN)" is not just for fun, it is absolutely essential.

I totally agree with you. But the issue is with IP cores which has this feature implemented in.
Even when user does not want to use that feature but he bought IP with configuration supported RSC, then he has to switch off IP alignment.
There is no IP alignment with RSC in the GEM:
"When the gem rsc define has been set the receive buffer offset cannot be changed in the network configuration register."
If IP supports RSC and skb has 2B reserved for alignment we end up with none packets receive correctly (2B missing in the each skb).
We can either leave few customers without support in Linux driver or let them use the driver with decrease performance.

Re: [PATCH 0/3] Receive Side Coalescing for macb driver

[David Miller]

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Tue, 17 Apr 2018 08:59:35 +0000

> If IP supports RSC and skb has 2B reserved for alignment we end up
> with none packets receive correctly (2B missing in the each skb).
> We can either leave few customers without support in Linux driver or
> let them use the driver with decrease performance.

You probably want to find a way to memmove() the headers to be aligned
in this case, and then point the data afterwards into SKB page frags.