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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2016 09:13:22 +0000
From:   Salil Mehta <salil.mehta@...wei.com>
To:     David Miller <davem@...emloft.net>
CC:     "Zhuangyuzeng (Yisen)" <yisen.zhuang@...wei.com>,
        "mehta.salil.lnk@...il.com" <mehta.salil.lnk@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH net-next] net: hns: Fix to conditionally convey RX
 checksum flag to stack

> -----Original Message-----
> From: David Miller [mailto:davem@...emloft.net]
> Sent: Monday, November 28, 2016 5:13 PM
> To: Salil Mehta
> Cc: Zhuangyuzeng (Yisen); mehta.salil.lnk@...il.com;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next] net: hns: Fix to conditionally convey RX
> checksum flag to stack
> 
> From: Salil Mehta <salil.mehta@...wei.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