dpaa_eth: Update PR processing routine on Rx path

Make several changes to the logic of the parse results processing
function on the Rx path.

Currently all frames with a parser header error are received on the
error frame queue, so the processing of PR on the default rx path
doesn't need to check for header errors.
Remove both the checks and a counter that was supposed to count
frames with an invalid L4 header.

Change-Id: Ibe7eb9957e1a0c78abf8445f758067340b48cd86
Signed-off-by: Ioana Radulescu <ruxandra.radulescu@freescale.com>
Reviewed-on: http://git.am.freescale.net:8181/1279
Reviewed-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>
Tested-by: Fleming Andrew-AFLEMING <AFLEMING@freescale.com>

3 files changed, 52 insertions, 110 deletions

diff --git a/drivers/net/ethernet/freescale/dpa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpa/dpaa_eth.c
index 8662b57..20aaa7f 100644
--- a/drivers/net/ethernet/freescale/dpa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpa/dpaa_eth.c
@@ -1205,52 +1205,28 @@ static void _dpa_tx_error(struct net_device		*net_dev,
  * the integrity of the frame, its checksum, the length of the parsed headers
  * and whether the frame is suitable for GRO.
  *
+ * Assumes no parser errors, since any error frame is dropped before this
+ * function is called.
+ *
  * @skb		will have its ip_summed field overwritten;
  * @use_gro	will only be written with 0, if the frame is definitely not
  *		GRO-able; otherwise, it will be left unchanged;
  * @hdr_size	will be written with a safe value, at least the size of the
  *		headers' length.
- *
- * Returns 0 if the frame contained no detectable error (including if the FMan
- * Parser has not in fact been running), and a non-zero value if the Parser
- * has run but encountered an error.
  */
-int __hot _dpa_process_parse_results(const t_FmPrsResult *parse_results,
+void __hot _dpa_process_parse_results(const t_FmPrsResult *parse_results,
 	const struct qm_fd *fd,
 	struct sk_buff *skb,
 	int *use_gro,
 	unsigned int *hdr_size __maybe_unused)
 {
-	if (likely(fm_l4_hxs_has_run(parse_results))) {
-		/*
-		 * Was there any parsing error? Note: this includes the check
-		 * for a valid L4 checksum.
-		 */
-		if (unlikely(fm_l4_hxs_error(parse_results)))
-			/* Leave it to the caller to handle the frame. */
-			return parse_results->l4r;
-
-#ifdef CONFIG_DPAA_ETH_SG_SUPPORT
-		/*
-		 * If the HXS Parser has successfully run, we can reduce the
-		 * number of bytes we'll memcopy into skb->data.
-		 */
-		*hdr_size = parse_results->nxthdr_off;
-#endif
+	if (fd->status & FM_FD_STAT_L4CV) {
 		/*
-		 * We know the frame is valid. But has the L4 checksum actually
-		 * been validated? (The L4CV bit is only set if the frame is
-		 * TCP or UDP-with-non-zero-csum.)
+		 * The parser has run and performed L4 checksum validation.
+		 * We know there were no parser errors (and implicitly no
+		 * L4 csum error), otherwise we wouldn't be here.
 		 */
-		if (fd->status & FM_FD_STAT_L4CV)
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
-		else
-			/*
-			 * If it turns out to be a 0-csum UDP, the stack will
-			 * figure it out itself later, sparing us an extra
-			 * check here on the fastpath of every incoming frame.
-			 */
-			skb->ip_summed = CHECKSUM_NONE;
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		/*
 		 * Don't go through GRO for certain types of traffic that
@@ -1263,27 +1239,38 @@ int __hot _dpa_process_parse_results(const t_FmPrsResult *parse_results,
 		 */
 		if (!fm_l4_frame_is_tcp(parse_results))
 			*use_gro = 0;
-	} else {
-		/* Inform the stack that we haven't done any csum validation. */
-		skb->ip_summed = CHECKSUM_NONE;
+
 #ifdef CONFIG_DPAA_ETH_SG_SUPPORT
 		/*
-		 * Also, since the Parser hasn't run, we don't know the size of
-		 * the headers, so we fall back to a safe default.
+		 * If the L4 HXS Parser has successfully run, we can reduce the
+		 * number of bytes we'll memcopy into skb->data.
 		 */
+		*hdr_size = parse_results->nxthdr_off;
+#endif
+		return;
+	}
+
+	/*
+	 * We're here because either the parser didn't run or the L4 checksum
+	 * was not verified. This may include the case of a UDP frame with
+	 * checksum zero or an L4 proto other than TCP/UDP
+	 */
+	skb->ip_summed = CHECKSUM_NONE;
+#ifdef CONFIG_DPAA_ETH_SG_SUPPORT
+	/*
+	 * Even if checksum was not verified, it's still possible L4 parser
+	 * has run, in which case we know the headers size.
+	 * Otherwise we fall back to a safe default.
+	 */
+	if (fm_l4_hxs_has_run(parse_results))
+		*hdr_size = parse_results->nxthdr_off;
+	else
 		*hdr_size = min((ssize_t)DPA_COPIED_HEADERS_SIZE,
 				dpa_fd_length(fd));
 #endif
-		/*
-		 * Bypass GRO for unknown traffic or if no PCDs are applied.
-		 * It's unlikely that a GRO handler is installed for this proto
-		 * or, if it is, user does not seem to care about performance
-		 * (otherwise, PCDs would have been in place).
-		 */
-		*use_gro = 0;
-	}
 
-	return 0;
+	/* Bypass GRO for unknown traffic or if no PCDs are applied */
+	*use_gro = 0;
 }
 
 #ifndef CONFIG_DPAA_ETH_SG_SUPPORT
@@ -1357,13 +1344,9 @@ void __hot _dpa_rx(struct net_device *net_dev,
 
 	/* Validate the skb csum and figure out whether GRO is appropriate */
 	parse_result = (t_FmPrsResult *)((u8 *)skbh + DPA_RX_PRIV_DATA_SIZE);
-	ret = _dpa_process_parse_results(parse_result, fd, skb, &use_gro,
+	_dpa_process_parse_results(parse_result, fd, skb, &use_gro,
 					 &hdr_size_unused);
-	if (unlikely(ret)) {
-		percpu_priv->l4_hxs_errors++;
-		percpu_priv->stats.rx_dropped++;
-		goto drop_invalid_frame;
-	}
+
 	if (use_gro) {
 		gro_result_t gro_result;
 
@@ -1384,7 +1367,6 @@ packet_dropped:
 skb_stolen:
 	return;
 
-drop_invalid_frame:
 drop_large_frame:
 	dev_kfree_skb(skb);
 	return;
@@ -3071,7 +3053,7 @@ static int __cold dpa_debugfs_show(struct seq_file *file, void *offset)
 	/* "Standard" counters */
 	seq_printf(file, "\nDPA counters for %s:\n"
 		"CPU           irqs        rx        tx   recycle" \
-		"   confirm     tx sg    tx err    rx err   l4 hxs drp    bp count\n",
+		"   confirm     tx sg    tx err    rx err   bp count\n",
 		priv->net_dev->name);
 	for_each_online_cpu(i) {
 		percpu_priv = per_cpu_ptr(priv->percpu_priv, i);
@@ -3089,11 +3071,10 @@ static int __cold dpa_debugfs_show(struct seq_file *file, void *offset)
 		total.tx_frag_skbuffs += percpu_priv->tx_frag_skbuffs;
 		total.stats.tx_errors += percpu_priv->stats.tx_errors;
 		total.stats.rx_errors += percpu_priv->stats.rx_errors;
-		total.l4_hxs_errors += percpu_priv->l4_hxs_errors;
 		count_total += dpa_bp_count;
 
 		seq_printf(file, "     %hu/%hu  %8u  %8lu  %8lu  %8u  %8u" \
-				"  %8u  %8lu  %8lu     %8u    %8d\n",
+				"  %8u  %8lu  %8lu     %8d\n",
 				get_hard_smp_processor_id(i), i,
 				percpu_priv->in_interrupt,
 				percpu_priv->stats.rx_packets,
@@ -3103,11 +3084,10 @@ static int __cold dpa_debugfs_show(struct seq_file *file, void *offset)
 				percpu_priv->tx_frag_skbuffs,
 				percpu_priv->stats.tx_errors,
 				percpu_priv->stats.rx_errors,
-				percpu_priv->l4_hxs_errors,
 				dpa_bp_count);
 	}
 	seq_printf(file, "Total     %8u  %8lu  %8lu  %8u  %8u  %8u  %8lu  %8lu"\
-				"     %8u    %8d\n",
+				"     %8d\n",
 			total.in_interrupt,
 			total.stats.rx_packets,
 			total.stats.tx_packets,
@@ -3116,7 +3096,6 @@ static int __cold dpa_debugfs_show(struct seq_file *file, void *offset)
 			total.tx_frag_skbuffs,
 			total.stats.tx_errors,
 			total.stats.rx_errors,
-			total.l4_hxs_errors,
 			count_total);
 
 	/* Congestion stats */
diff --git a/drivers/net/ethernet/freescale/dpa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpa/dpaa_eth.h
index c12f90d..5b7280b 100644
--- a/drivers/net/ethernet/freescale/dpa/dpaa_eth.h
+++ b/drivers/net/ethernet/freescale/dpa/dpaa_eth.h
@@ -339,13 +339,6 @@ struct dpa_percpu_priv_s {
 	u32 tx_confirm;
 	/* fragmented (non-linear) skbuffs received from the stack */
 	u32 tx_frag_skbuffs;
-	/*
-	 * Frames identified as L4 packets (by FMan's Hardware Parser, but for
-	 * which the parsing failed due to some error condition. If we come
-	 * across such frames, we drop them instead of passing them up the
-	 * stack, which means the L4 stats in the stack won't increment.
-	 */
-	u32 l4_hxs_errors;
 	struct net_device_stats	 stats;
 	struct dpa_rx_errors rx_errors;
 	struct dpa_ern_cnt ern_cnt;
@@ -419,7 +412,7 @@ int __hot dpa_tx(struct sk_buff *skb, struct net_device *net_dev);
 struct sk_buff *_dpa_cleanup_tx_fd(const struct dpa_priv_s *priv,
 				   const struct qm_fd *fd);
 
-int __hot _dpa_process_parse_results(const t_FmPrsResult *parse_results,
+void __hot _dpa_process_parse_results(const t_FmPrsResult *parse_results,
 				     const struct qm_fd *fd,
 				     struct sk_buff *skb,
 				     int *use_gro,
diff --git a/drivers/net/ethernet/freescale/dpa/dpaa_eth_sg.c b/drivers/net/ethernet/freescale/dpa/dpaa_eth_sg.c
index 95c1996..7e283d4 100644
--- a/drivers/net/ethernet/freescale/dpa/dpaa_eth_sg.c
+++ b/drivers/net/ethernet/freescale/dpa/dpaa_eth_sg.c
@@ -282,11 +282,8 @@ struct sk_buff *_dpa_cleanup_tx_fd(const struct dpa_priv_s *priv,
  *
  * If the entire frame fits in the skb linear buffer, the page holding the
  * received data is recycled as it is no longer required.
- *
- * Return 0 if the ingress skb was properly constructed, non-zero if an error
- * was encountered and the frame should be dropped.
  */
-static int __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
+static void __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
 	const struct qm_fd *fd, struct sk_buff *skb, int *use_gro)
 {
 	unsigned int copy_size = DPA_COPIED_HEADERS_SIZE;
@@ -297,7 +294,6 @@ static int __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
 	struct dpa_bp *dpa_bp = priv->dpa_bp;
 	unsigned char *tailptr;
 	const t_FmPrsResult *parse_results;
-	int ret;
 
 	vaddr = phys_to_virt(addr);
 
@@ -306,13 +302,9 @@ static int __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
 		dpa_ptp_store_rxstamp(priv->net_dev, skb, fd);
 #endif
 
-	/* Peek at the parse results for frame validation. */
+	/* Peek at the parse results for csum validation and headers size */
 	parse_results = (const t_FmPrsResult *)(vaddr + DPA_RX_PRIV_DATA_SIZE);
-	ret = _dpa_process_parse_results(parse_results, fd, skb, use_gro,
-		&copy_size);
-	if (unlikely(ret))
-		/* This is definitely a bad frame, don't go further. */
-		return ret;
+	_dpa_process_parse_results(parse_results, fd, skb, use_gro, &copy_size);
 
 	tailptr = skb_put(skb, copy_size);
 
@@ -339,8 +331,6 @@ static int __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
 		/* recycle the page */
 		dpa_bp_add_page(dpa_bp, (unsigned long)vaddr);
 	}
-
-	return 0;
 }
 
 
@@ -351,7 +341,7 @@ static int __hot contig_fd_to_skb(const struct dpa_priv_s *priv,
  *
  * The page holding the S/G Table is recycled here.
  */
-static int __hot sg_fd_to_skb(const struct dpa_priv_s *priv,
+static void __hot sg_fd_to_skb(const struct dpa_priv_s *priv,
 			       const struct qm_fd *fd, struct sk_buff *skb,
 			       int *use_gro)
 {
@@ -363,7 +353,7 @@ static int __hot sg_fd_to_skb(const struct dpa_priv_s *priv,
 	struct page *page;
 	int frag_offset, frag_len;
 	int page_offset;
-	int i, ret;
+	int i;
 	unsigned int copy_size = DPA_COPIED_HEADERS_SIZE;
 	const t_FmPrsResult *parse_results;
 
@@ -373,12 +363,8 @@ static int __hot sg_fd_to_skb(const struct dpa_priv_s *priv,
 	 * in the buffer containing the sgt.
 	 */
 	parse_results = (const t_FmPrsResult *)(vaddr + DPA_RX_PRIV_DATA_SIZE);
-	/* Validate the frame before anything else. */
-	ret = _dpa_process_parse_results(parse_results, fd, skb, use_gro,
-		&copy_size);
-	if (unlikely(ret))
-		/* Bad frame, stop processing now. */
-		return ret;
+	/* Inspect the parse results before anything else. */
+	_dpa_process_parse_results(parse_results, fd, skb, use_gro, &copy_size);
 
 	/*
 	 * Iterate through the SGT entries and add the data buffers as
@@ -443,8 +429,6 @@ static int __hot sg_fd_to_skb(const struct dpa_priv_s *priv,
 	dpa_bp = dpa_bpid2pool(fd->bpid);
 	BUG_ON(IS_ERR(dpa_bp));
 	dpa_bp_add_page(dpa_bp, (unsigned long)vaddr);
-
-	return 0;
 }
 
 void __hot _dpa_rx(struct net_device *net_dev,
@@ -499,25 +483,11 @@ void __hot _dpa_rx(struct net_device *net_dev,
 	/* prefetch the first 64 bytes of the frame or the SGT start */
 	prefetch(phys_to_virt(addr) + dpa_fd_offset(fd));
 
-	if (likely(fd->format == qm_fd_contig)) {
-		if (unlikely(contig_fd_to_skb(priv, fd, skb, &use_gro))) {
-			/*
-			 * There was a L4 HXS error - e.g. the L4 csum was
-			 * invalid - so drop the frame early instead of passing
-			 * it on to the stack. We'll increment our private
-			 * counters to track this event.
-			 */
-			percpu_priv->l4_hxs_errors++;
-			percpu_stats->rx_dropped++;
-			goto drop_bad_frame;
-		}
-	} else if (fd->format == qm_fd_sg) {
-		if (unlikely(sg_fd_to_skb(priv, fd, skb, &use_gro))) {
-			percpu_priv->l4_hxs_errors++;
-			percpu_stats->rx_dropped++;
-			goto drop_bad_frame;
-		}
-	} else
+	if (likely(fd->format == qm_fd_contig))
+		contig_fd_to_skb(priv, fd, skb, &use_gro);
+	else if (fd->format == qm_fd_sg)
+		sg_fd_to_skb(priv, fd, skb, &use_gro);
+	else
 		/* The only FD types that we may receive are contig and S/G */
 		BUG();