netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Gregory CLEMENT <gregory.clement@bootlin.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Yelena Krivosheev <yelena@marvell.com>,
	"David S. Miller" <davem@davemloft.net>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Marcin Wojtas <mw@semihalf.com>,
	netdev@vger.kernel.org
Subject: [query] about recent mvneta patches
Date: Fri, 10 Aug 2018 14:58:28 +0800	[thread overview]
Message-ID: <20180810145828.6d05b776@xhacker.debian> (raw)

Hi,

Today I have a look at recent mvneta patches in net-next, I worried two patches:

1. commit 562e2f467e71 ("net: mvneta: Improve the buffer allocation method
for SWBM") sets rx_offset_correction as 0 for SW BM, but IIRC, the offset
is introduced to support 64bit platforms. So in theory the commit could break
arm64 platforms with mvneta SW BM.

Fortunately, commit d93277b9839b ("Revert "arm64: Increase the max granular
size") makes arm64 L1_CACHE_BYTES equal to 64 again, thus NET_SKB_PAD is 64
too, so the commit 562e2f467e71 doesn't introduce regression, but it hides
the bug we tried to fix in commit 8d5047cf9ca ("net: mvneta: Convert to be 64
bits compatible)

IMHO, this patch need to be updated to not introduce regression even the
L1_CACHE_BYTES is larger than 64B, what do you think?

2. commit f945cec88cb ("net: mvneta: Verify hardware checksum only when
offload checksum feature is set"), I agree with the point. But
MVNETA_RX_CSUM_WITH_PSEUDO_HDR bit is always set, so the RX CSUM is always
generated no matter we set NETIF_F_RXCSUM or not, so IMHO, we should
always set NETIF_F_RXCSUM. And since we enabled GRO, so what about something
as below:

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index bc80a678abc3..a5043b27bf37 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4598,7 +4598,8 @@ static int mvneta_probe(struct platform_device *pdev)
 		}
 	}
 
-	dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_TSO;
+	dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+			NETIF_F_RXCSUM | NETIF_F_TSO | NETIF_F_GRO;
 	dev->hw_features |= dev->features;
 	dev->vlan_features |= dev->features;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;

                 reply	other threads:[~2018-08-10  9:30 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180810145828.6d05b776@xhacker.debian \
    --to=jisheng.zhang@synaptics.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=gregory.clement@bootlin.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mw@semihalf.com \
    --cc=netdev@vger.kernel.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=yelena@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).