Skip to content

Commit 334e90b

Browse files
committed
Merge branch 'fix-large-frames-in-the-gemini-ethernet-driver'
Linus Walleij says: ==================== Fix large frames in the Gemini ethernet driver This is the result of a bug hunt for a problem with the RTL8366RB DSA switch leading me wrong all over the place. I am indebted to Vladimir Oltean who as usual pointed out where the real problem was, many thanks! Tryig to actually use big ("jumbo") frames on this hardware uncovered the real bugs. Then I tested it on the DSA switch and it indeed fixes the issue. To make sure it also works fine with big frames on non-DSA devices I also copied a large video file over scp to a device with maximum frame size, the data was transported in large TCP packets ending up in 0x7ff sized frames using software checksumming at ~2.0 MB/s. If I set down the MTU to the standard 1500 bytes so that hardware checksumming is used, the scp transfer of the same file was slightly lower, ~1.8-1.9 MB/s. Despite this not being the best test it shows that we can now stress the hardware with large frames and that software checksum works fine. v3: https://lore.kernel.org/r/20231107-gemini-largeframe-fix-v3-0-e3803c080b75@linaro.org v2: https://lore.kernel.org/r/20231105-gemini-largeframe-fix-v2-0-cd3a5aa6c496@linaro.org v1: https://lore.kernel.org/r/20231104-gemini-largeframe-fix-v1-0-9c5513f22f33@linaro.org ==================== Link: https://lore.kernel.org/r/20231109-gemini-largeframe-fix-v4-0-6e611528db08@linaro.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 3cffa2d + dc6c0bf commit 334e90b

2 files changed

Lines changed: 31 additions & 18 deletions

File tree

drivers/net/ethernet/cortina/gemini.c

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,8 @@ static const struct gmac_max_framelen gmac_maxlens[] = {
432432
.val = CONFIG0_MAXLEN_1536,
433433
},
434434
{
435-
.max_l3_len = 1542,
436-
.val = CONFIG0_MAXLEN_1542,
435+
.max_l3_len = 1548,
436+
.val = CONFIG0_MAXLEN_1548,
437437
},
438438
{
439439
.max_l3_len = 9212,
@@ -1145,6 +1145,7 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
11451145
dma_addr_t mapping;
11461146
unsigned short mtu;
11471147
void *buffer;
1148+
int ret;
11481149

11491150
mtu = ETH_HLEN;
11501151
mtu += netdev->mtu;
@@ -1159,9 +1160,30 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
11591160
word3 |= mtu;
11601161
}
11611162

1162-
if (skb->ip_summed != CHECKSUM_NONE) {
1163+
if (skb->len >= ETH_FRAME_LEN) {
1164+
/* Hardware offloaded checksumming isn't working on frames
1165+
* bigger than 1514 bytes. A hypothesis about this is that the
1166+
* checksum buffer is only 1518 bytes, so when the frames get
1167+
* bigger they get truncated, or the last few bytes get
1168+
* overwritten by the FCS.
1169+
*
1170+
* Just use software checksumming and bypass on bigger frames.
1171+
*/
1172+
if (skb->ip_summed == CHECKSUM_PARTIAL) {
1173+
ret = skb_checksum_help(skb);
1174+
if (ret)
1175+
return ret;
1176+
}
1177+
word1 |= TSS_BYPASS_BIT;
1178+
} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
11631179
int tcp = 0;
11641180

1181+
/* We do not switch off the checksumming on non TCP/UDP
1182+
* frames: as is shown from tests, the checksumming engine
1183+
* is smart enough to see that a frame is not actually TCP
1184+
* or UDP and then just pass it through without any changes
1185+
* to the frame.
1186+
*/
11651187
if (skb->protocol == htons(ETH_P_IP)) {
11661188
word1 |= TSS_IP_CHKSUM_BIT;
11671189
tcp = ip_hdr(skb)->protocol == IPPROTO_TCP;
@@ -1978,15 +2000,6 @@ static int gmac_change_mtu(struct net_device *netdev, int new_mtu)
19782000
return 0;
19792001
}
19802002

1981-
static netdev_features_t gmac_fix_features(struct net_device *netdev,
1982-
netdev_features_t features)
1983-
{
1984-
if (netdev->mtu + ETH_HLEN + VLAN_HLEN > MTU_SIZE_BIT_MASK)
1985-
features &= ~GMAC_OFFLOAD_FEATURES;
1986-
1987-
return features;
1988-
}
1989-
19902003
static int gmac_set_features(struct net_device *netdev,
19912004
netdev_features_t features)
19922005
{
@@ -2212,7 +2225,6 @@ static const struct net_device_ops gmac_351x_ops = {
22122225
.ndo_set_mac_address = gmac_set_mac_address,
22132226
.ndo_get_stats64 = gmac_get_stats64,
22142227
.ndo_change_mtu = gmac_change_mtu,
2215-
.ndo_fix_features = gmac_fix_features,
22162228
.ndo_set_features = gmac_set_features,
22172229
};
22182230

@@ -2464,11 +2476,12 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
24642476

24652477
netdev->hw_features = GMAC_OFFLOAD_FEATURES;
24662478
netdev->features |= GMAC_OFFLOAD_FEATURES | NETIF_F_GRO;
2467-
/* We can handle jumbo frames up to 10236 bytes so, let's accept
2468-
* payloads of 10236 bytes minus VLAN and ethernet header
2479+
/* We can receive jumbo frames up to 10236 bytes but only
2480+
* transmit 2047 bytes so, let's accept payloads of 2047
2481+
* bytes minus VLAN and ethernet header
24692482
*/
24702483
netdev->min_mtu = ETH_MIN_MTU;
2471-
netdev->max_mtu = 10236 - VLAN_ETH_HLEN;
2484+
netdev->max_mtu = MTU_SIZE_BIT_MASK - VLAN_ETH_HLEN;
24722485

24732486
port->freeq_refill = 0;
24742487
netif_napi_add(netdev, &port->napi, gmac_napi_poll);

drivers/net/ethernet/cortina/gemini.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ union gmac_txdesc_3 {
502502
#define SOF_BIT 0x80000000
503503
#define EOF_BIT 0x40000000
504504
#define EOFIE_BIT BIT(29)
505-
#define MTU_SIZE_BIT_MASK 0x1fff
505+
#define MTU_SIZE_BIT_MASK 0x7ff /* Max MTU 2047 bytes */
506506

507507
/* GMAC Tx Descriptor */
508508
struct gmac_txdesc {
@@ -787,7 +787,7 @@ union gmac_config0 {
787787
#define CONFIG0_MAXLEN_1536 0
788788
#define CONFIG0_MAXLEN_1518 1
789789
#define CONFIG0_MAXLEN_1522 2
790-
#define CONFIG0_MAXLEN_1542 3
790+
#define CONFIG0_MAXLEN_1548 3
791791
#define CONFIG0_MAXLEN_9k 4 /* 9212 */
792792
#define CONFIG0_MAXLEN_10k 5 /* 10236 */
793793
#define CONFIG0_MAXLEN_1518__6 6

0 commit comments

Comments
 (0)