[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201017210235.GU456889@lunn.ch>
Date: Sat, 17 Oct 2020 23:02:35 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Alexander A Sverdlin <alexander.sverdlin@...ia.com>
Cc: devel@...verdev.osuosl.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Aaro Koskinen <aaro.koskinen@....fi>,
Ralf Baechle <ralf@...ux-mips.org>,
"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net] staging: octeon: Drop on uncorrectable alignment
or FCS error
> diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> index 2c16230..9ebd665 100644
> --- a/drivers/staging/octeon/ethernet-rx.c
> +++ b/drivers/staging/octeon/ethernet-rx.c
> @@ -69,15 +69,17 @@ static inline int cvm_oct_check_rcv_error(struct cvmx_wqe *work)
> else
> port = work->word1.cn38xx.ipprt;
>
> - if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64)) {
> + if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64))
It would be nice to replace all these err_code magic numbers with #defines.
You should also replace 64 with ETH_ZLEN + ETH_FCS_LEN. I also wonder
if <= should be just < ?
> /*
> * Ignore length errors on min size packets. Some
> * equipment incorrectly pads packets to 64+4FCS
> * instead of 60+4FCS. Note these packets still get
> * counted as frame errors.
> */
> - } else if (work->word2.snoip.err_code == 5 ||
> - work->word2.snoip.err_code == 7) {
> + return 0;
> +
> + if (work->word2.snoip.err_code == 5 ||
> + work->word2.snoip.err_code == 7) {
> /*
> * We received a packet with either an alignment error
> * or a FCS error. This may be signalling that we are
> @@ -108,7 +110,10 @@ static inline int cvm_oct_check_rcv_error(struct cvmx_wqe *work)
> /* Port received 0xd5 preamble */
> work->packet_ptr.s.addr += i + 1;
> work->word1.len -= i + 5;
> - } else if ((*ptr & 0xf) == 0xd) {
> + return 0;
> + }
> +
> + if ((*ptr & 0xf) == 0xd) {
The comments are not so clear what is going on here. Can this
incorrectly match a destination MAC address of xD:XX:XX:XX:XX:XX.
> /* Port received 0xd preamble */
> work->packet_ptr.s.addr += i;
> work->word1.len -= i + 4;
Andrew
Powered by blists - more mailing lists