RE: [PATCH net-next] net: hns: Fix to conditionally convey RX checksum flag to stack

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Monday, November 28, 2016 5:13 PM
To: Salil Mehta
Cc: Zhuangyuzeng (Yisen); mehta.salil.lnk@gmail.com;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
Subject: Re: [PATCH net-next] net: hns: Fix to conditionally convey RX
checksum flag to stack

From: Salil Mehta <redacted>
Date: Fri, 25 Nov 2016 13:32:40 +0000

quoted

@@ -778,6 +778,35 @@ int hns_ae_get_regs_len(struct hnae_handle

*handle)

quoted

 	return total_num;
 }

+static bool hns_ae_is_l3l4_csum_err(struct hnae_handle *handle)
+{
+	struct hns_ppe_cb *ppe_cb = hns_get_ppe_cb(handle);
+	u32 regval;
+	bool retval = false;
+
+	/* read PPE_HIS_PRO_ERR register and check for the checksum

errors */

quoted

+	regval = dsaf_read_dev(ppe_cb, PPE_HIS_PRO_ERR_REG);
+

I don't see how a single register can properly provide error status for
a ring
of pending received packets.

No matter how this register is implemented, it is either going to
result in
packets erroneously being marked as having errors, or error status
being
lost when multiple packets in a row have such errors.

For example, if you receive several packets in a row that have errors,
you'll read this register for the first one.  If this read clears the
error
status, which I am guessing it does, then you won't see the error
status
for the next packet that had one of these errors as well.

Agreed David. I think I missed this part. This register is 
not well thought of and looks useless for checksum. Thanks
for identifying this!

If you don't have something which is provided on a per-packet basis
then you can't determine the error properly.  Therefore you will just
have to always ignore the checksum if there is any error indicated in
the ring descriptor.

Yes, will float another patch ignoring the checksum.

Thanks
Salil