[PATCH net 0/2] mlx4 fixes for 4.20-rc

STALE2852d

Revision v1 of 2 in this series.

10 messages, 5 authors, 2018-12-05 · open the first message on its own page

[PATCH net 0/2] mlx4 fixes for 4.20-rc

From: Tariq Toukan <hidden>
Date: 2018-12-02 12:34:51

Hi Dave,

This patchset includes small fixes for the mlx4_en driver.

First patch by Eran fixes the value used to init the netdevice's
min_mtu field.
Please queue it to -stable >= v4.10.

Second patch by Saeed adds missing Kconfig build dependencies.

Series generated against net commit:
35b827b6d061 tun: forbid iface creation with rtnl ops

Thanks,
Tariq.


Eran Ben Elisha (1):
  net/mlx4_en: Change min MTU size to ETH_MIN_MTU

Saeed Mahameed (1):
  net/mlx4_en: Fix build break when CONFIG_INET is off

 drivers/net/ethernet/mellanox/mlx4/Kconfig     | 2 +-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 4 ++--
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

-- 
1.8.3.1

[PATCH net 2/2] net/mlx4_en: Fix build break when CONFIG_INET is off

From: Tariq Toukan <hidden>
Date: 2018-12-02 12:34:51

From: Saeed Mahameed <redacted>

MLX4_EN depends on NETDEVICES, ETHERNET and INET Kconfigs.
Make sure they are listed in MLX4_EN Kconfig dependencies.

This fixes the following build break:

drivers/net/ethernet/mellanox/mlx4/en_rx.c:582:18: warning: ‘struct iphdr’ declared inside parameter list [enabled by default]
struct iphdr *iph)
^
drivers/net/ethernet/mellanox/mlx4/en_rx.c:582:18: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
drivers/net/ethernet/mellanox/mlx4/en_rx.c: In function ‘get_fixed_ipv4_csum’:
drivers/net/ethernet/mellanox/mlx4/en_rx.c:586:20: error: dereferencing pointer to incomplete type
_u8 ipproto = iph->protocol;

Signed-off-by: Saeed Mahameed <redacted>
Signed-off-by: Tariq Toukan <redacted>
---
 drivers/net/ethernet/mellanox/mlx4/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 36054e6fb9d3..f200b8c420d5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -5,7 +5,7 @@
 config MLX4_EN
 	tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
 	depends on MAY_USE_DEVLINK
-	depends on PCI
+	depends on PCI && NETDEVICES && ETHERNET && INET
 	select MLX4_CORE
 	imply PTP_1588_CLOCK
 	---help---
-- 
1.8.3.1

[PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: Tariq Toukan <hidden>
Date: 2018-12-02 12:34:51

From: Eran Ben Elisha <redacted>

NIC driver minimal MTU size shall be set to ETH_MIN_MTU, as defined in
the RFC791 and in the network stack. Remove old mlx4_en only define for
it, which was set to wrong value.

Fixes: b80f71f5816f ("ethernet/mellanox: use core min/max MTU checking")
Signed-off-by: Eran Ben Elisha <redacted>
Signed-off-by: Tariq Toukan <redacted>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 4 ++--
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b744cd49a785..6b88881b8e35 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3493,8 +3493,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 		dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
 	}
 
-	/* MTU range: 46 - hw-specific max */
-	dev->min_mtu = MLX4_EN_MIN_MTU;
+	/* MTU range: 68 - hw-specific max */
+	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = priv->max_mtu;
 
 	mdev->pndev[port] = dev;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 485d856546c6..8137454e2534 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -161,7 +161,6 @@
 #define MLX4_SELFTEST_LB_MIN_MTU (MLX4_LOOPBACK_TEST_PAYLOAD + NET_IP_ALIGN + \
 				  ETH_HLEN + PREAMBLE_LEN)
 
-#define MLX4_EN_MIN_MTU		46
 /* VLAN_HLEN is added twice,to support skb vlan tagged with multiple
  * headers. (For example: ETH_P_8021Q and ETH_P_8021AD).
  */
-- 
1.8.3.1

Re: [PATCH net 0/2] mlx4 fixes for 4.20-rc

From: David Miller <davem@davemloft.net>
Date: 2018-12-04 00:17:18

From: Tariq Toukan <redacted>
Date: Sun,  2 Dec 2018 14:34:35 +0200
This patchset includes small fixes for the mlx4_en driver.

First patch by Eran fixes the value used to init the netdevice's
min_mtu field.
Please queue it to -stable >= v4.10.

Second patch by Saeed adds missing Kconfig build dependencies.

Series generated against net commit:
35b827b6d061 tun: forbid iface creation with rtnl ops
Series applied and patch #1 queued up for -stable, thanks.

RE: [PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: David Laight <hidden>
Date: 2018-12-04 16:59:11

From: Tariq Toukan
Sent: 02 December 2018 12:35
From: Eran Ben Elisha <redacted>

NIC driver minimal MTU size shall be set to ETH_MIN_MTU, as defined in
the RFC791 and in the network stack. Remove old mlx4_en only define for
it, which was set to wrong value.
...
-	/* MTU range: 46 - hw-specific max */
-	dev->min_mtu = MLX4_EN_MIN_MTU;
+	/* MTU range: 68 - hw-specific max */
+	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = priv->max_mtu;
Where does 68 come from?
The minimum size of an ethernet packet including the mac addresses
and CRC is 64 bytes - but that would never be an 'mtu'.

Since 64 - 46 = 18, the 46 probably excludes both MAC addresses,
the ethertype/length and the CRC.
This is 'sort of' the minimum mtu for an ethernet frame.

I'm not sure which values are supposed to be in dev->min/max_mtu.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: Eric Dumazet <hidden>
Date: 2018-12-04 17:04:20


On 12/04/2018 08:59 AM, David Laight wrote:
From: Tariq Toukan
quoted
Sent: 02 December 2018 12:35
From: Eran Ben Elisha <redacted>

NIC driver minimal MTU size shall be set to ETH_MIN_MTU, as defined in
the RFC791 and in the network stack. Remove old mlx4_en only define for
it, which was set to wrong value.
...
quoted
-	/* MTU range: 46 - hw-specific max */
-	dev->min_mtu = MLX4_EN_MIN_MTU;
+	/* MTU range: 68 - hw-specific max */
+	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = priv->max_mtu;
Where does 68 come from?
Min IPv4 MTU per RFC791
The minimum size of an ethernet packet including the mac addresses
and CRC is 64 bytes - but that would never be an 'mtu'.

Since 64 - 46 = 18, the 46 probably excludes both MAC addresses,
the ethertype/length and the CRC.
This is 'sort of' the minimum mtu for an ethernet frame.

I'm not sure which values are supposed to be in dev->min/max_mtu.
I am not sure we really care, only syzkaller can possibly.

RE: [PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: David Laight <hidden>
Date: 2018-12-04 17:17:14

From: Eric Dumazet
Sent: 04 December 2018 17:04

On 12/04/2018 08:59 AM, David Laight wrote:
quoted
From: Tariq Toukan
quoted
Sent: 02 December 2018 12:35
From: Eran Ben Elisha <redacted>

NIC driver minimal MTU size shall be set to ETH_MIN_MTU, as defined in
the RFC791 and in the network stack. Remove old mlx4_en only define for
it, which was set to wrong value.
...
quoted
-	/* MTU range: 46 - hw-specific max */
-	dev->min_mtu = MLX4_EN_MIN_MTU;
+	/* MTU range: 68 - hw-specific max */
+	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = priv->max_mtu;
Where does 68 come from?
Min IPv4 MTU per RFC791
Which has nothing to do with an ethernet driver.
Indeed, IIRC, it is the smallest maximum frame size that IPv4
can work over.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: David Laight <hidden>
Date: 2018-12-04 17:21:07

From: Eric Dumazet
Sent: 04 December 2018 17:04
On 12/04/2018 08:59 AM, David Laight wrote:
quoted
From: Tariq Toukan
quoted
Sent: 02 December 2018 12:35
From: Eran Ben Elisha <redacted>

NIC driver minimal MTU size shall be set to ETH_MIN_MTU, as defined in
the RFC791 and in the network stack. Remove old mlx4_en only define for
it, which was set to wrong value.
...
quoted
-	/* MTU range: 46 - hw-specific max */
-	dev->min_mtu = MLX4_EN_MIN_MTU;
+	/* MTU range: 68 - hw-specific max */
+	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = priv->max_mtu;
Where does 68 come from?
Min IPv4 MTU per RFC791
Maybe I'm just confused and these are the ranges that the 'maximum mtu'
can be set to.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH net 1/2] net/mlx4_en: Change min MTU size to ETH_MIN_MTU

From: Cong Wang <hidden>
Date: 2018-12-05 05:24:23

On Tue, Dec 4, 2018 at 9:06 AM Eric Dumazet [off-list ref] wrote:


On 12/04/2018 08:59 AM, David Laight wrote:
quoted
Where does 68 come from?
Min IPv4 MTU per RFC791
What's wrong with using IPV4_MIN_MTU instead of 68
even in just comment?

Re: [PATCH net 2/2] net/mlx4_en: Fix build break when CONFIG_INET is off

From: Cong Wang <hidden>
Date: 2018-12-05 05:40:47

On Sun, Dec 2, 2018 at 4:37 AM Tariq Toukan [off-list ref] wrote:
From: Saeed Mahameed <redacted>

MLX4_EN depends on NETDEVICES, ETHERNET and INET Kconfigs.
Make sure they are listed in MLX4_EN Kconfig dependencies.

This fixes the following build break:

drivers/net/ethernet/mellanox/mlx4/en_rx.c:582:18: warning: ‘struct iphdr’ declared inside parameter list [enabled by default]
struct iphdr *iph)
^
drivers/net/ethernet/mellanox/mlx4/en_rx.c:582:18: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
drivers/net/ethernet/mellanox/mlx4/en_rx.c: In function ‘get_fixed_ipv4_csum’:
drivers/net/ethernet/mellanox/mlx4/en_rx.c:586:20: error: dereferencing pointer to incomplete type
_u8 ipproto = iph->protocol;
I am confused with this build error. It complains about the
compiler doesn't know struct iphdr, but struct iphdr is defined
merely in include/uapi/linux/ip.h, without any dependency with
CONFIG_INET.

How could changing Kconfig fix this?

BTW, the patch itself is probably correct as mlx4 does rely on
INET to function, only the build failure confuses me.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help