Re: [PATCH net-next v3 2/2] stmmac: Fix kernel crashes for jumbo frames
From: Ben Hutchings <hidden>
Date: 2014-01-14 19:53:36
Also in:
linux-devicetree
On Tue, 2014-01-14 at 11:17 -0600, Vince Bridgers wrote:
These changes correct the following issues with jumbo frames on the stmmac driver: 1) The Synopsys EMAC can be configured to support different FIFO sizes at core configuration time. There's no way to query the controller and know the FIFO size, so the driver needs to get this information from the device tree in order to know how to correctly handle MTU changes and setting up dma buffers. The default max-frame-size is as currently used, which is the size of a jumbo frame. 2) The driver was enabling Jumbo frames by default, but was not allocating dma buffers of sufficient size to handle the maximum possible packet size that could be received. This led to memory corruption since DMAs were occurring beyond the extent of the allocated receive buffers for certain types of network traffic.
[...]
quoted hunk ↗ jump to hunk
--- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h@@ -293,6 +293,8 @@ struct dma_features { #define STMMAC_CHAIN_MODE 0x1 #define STMMAC_RING_MODE 0x2 +#define JUMBO_LEN 9000 + struct stmmac_desc_ops { /* DMA RX descriptor ring initialization */ void (*init_rx_desc) (struct dma_desc *p, int disable_rx_ic, int mode,
[...]
quoted hunk ↗ jump to hunk
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c@@ -51,6 +51,10 @@ static int stmmac_probe_config_dt(struct platform_device *pdev, plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(struct stmmac_mdio_bus_data), GFP_KERNEL); + /* Set the maxmtu to a default of 1500 in case the + * parameter is not present in the device tree + */ + plat->maxmtu = JUMBO_LEN;
The comment disagrees with the definition of JUMBO_LEN.
quoted hunk ↗ jump to hunk
/* * Currently only the properties needed on SPEAr600@@ -60,6 +64,7 @@ static int stmmac_probe_config_dt(struct platform_device *pdev, if (of_device_is_compatible(np, "st,spear600-gmac") || of_device_is_compatible(np, "snps,dwmac-3.70a") || of_device_is_compatible(np, "snps,dwmac")) { + of_property_read_u32(np, "max-frame-size", &plat->maxmtu);
[...] Is it the maximum frame size, including Ethernet header? (And then, is the CRC or any VLAN header included in the frame size?) Or is it the MTU, excluding all of those? You really need to be very clear about what this number represents. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.