> -----Original Message----- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, November 28, 2016 5:13 PM > To: Salil Mehta > Cc: Zhuangyuzeng (Yisen); mehta.salil....@gmail.com; > netdev@vger.kernel.org; linux-ker...@vger.kernel.org; Linuxarm > Subject: Re: [PATCH net-next] net: hns: Fix to conditionally convey RX > checksum flag to stack > > From: Salil Mehta <salil.me...@huawei.com> > Date: Fri, 25 Nov 2016 13:32:40 +0000 > > > @@ -778,6 +778,35 @@ int hns_ae_get_regs_len(struct hnae_handle > *handle) > > 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 */ > > + 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
