[PATCH net v2] ibmveth: Disable GSO for packets with small MSS

[PATCH net v2] ibmveth: Disable GSO for packets with small MSS

[Mingming Cao]

Some physical adapters on Power systems do not support segmentation
offload when the MSS is less than 224 bytes. Attempting to send such
packets causes the adapter to freeze, stopping all traffic until
manually reset.

Implement ndo_features_check to disable GSO for packets with small MSS
values. The network stack will perform software segmentation instead.

The 224-byte minimum matches ibmvnic
commit <f10b09ef687f> ("ibmvnic: Enforce stronger sanity checks
on GSO packets")
which uses the same physical adapters in SEA configurations.

Validated using iptables to force small MSS values. Without the fix,
the adapter freezes. With the fix, packets are segmented in software
and transmission succeeds.

Fixes: 8641dd85799f ("ibmveth: Add support for TSO")
Cc: stable@vger.kernel.org
Reviewed-by: Brian King <bjking1@linux.ibm.com>
Tested-by: Shaik Abdulla <shaik.abdulla1@ibm.com>
Tested-by: Naveed Ahmed <naveedaus@in.ibm.com>
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
---
v2: Add Fixes tag as requested by automated checks 

 drivers/net/ethernet/ibm/ibmveth.c | 20 ++++++++++++++++++++
 drivers/net/ethernet/ibm/ibmveth.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 58cc3147afe2..7935c9384ef4 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1756,6 +1756,25 @@ static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
 	return 0;
 }
 
+static netdev_features_t ibmveth_features_check(struct sk_buff *skb,
+						struct net_device *dev,
+						netdev_features_t features)
+{
+	/* Some physical adapters do not support segmentation offload with
+	 * MSS < 224. Disable GSO for such packets to avoid adapter freeze.
+	 */
+	if (skb_is_gso(skb)) {
+		if (skb_shinfo(skb)->gso_size < IBMVETH_MIN_LSO_MSS) {
+			netdev_warn_once(dev,
+					 "MSS %u too small for LSO, disabling GSO\n",
+					 skb_shinfo(skb)->gso_size);
+			features &= ~NETIF_F_GSO_MASK;
+		}
+	}
+
+	return features;
+}
+
 static const struct net_device_ops ibmveth_netdev_ops = {
 	.ndo_open		= ibmveth_open,
 	.ndo_stop		= ibmveth_close,
@@ -1767,6 +1786,7 @@ static const struct net_device_ops ibmveth_netdev_ops = {
 	.ndo_set_features	= ibmveth_set_features,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address    = ibmveth_set_mac_addr,
+	.ndo_features_check	= ibmveth_features_check,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= ibmveth_poll_controller,
 #endif
diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h
index 068f99df133e..d87713668ed3 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -37,6 +37,7 @@
 #define IBMVETH_ILLAN_IPV4_TCP_CSUM		0x0000000000000002UL
 #define IBMVETH_ILLAN_ACTIVE_TRUNK		0x0000000000000001UL
 
+#define IBMVETH_MIN_LSO_MSS		224	/* Minimum MSS for LSO */
 /* hcall macros */
 #define h_register_logical_lan(ua, buflst, rxq, fltlst, mac) \
   plpar_hcall_norets(H_REGISTER_LOGICAL_LAN, ua, buflst, rxq, fltlst, mac)
-- 
2.39.3 (Apple Git-146)

Re: [PATCH net v2] ibmveth: Disable GSO for packets with small MSS

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ibmveth: Disable GSO for packets with small MSS

This commit adds an ndo_features_check callback to the ibmveth driver to
disable segmentation offload when the MSS is less than 224 bytes. This
prevents physical adapters in SEA configurations from freezing when given
small MSS packets.

> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c

[ ... ]

> @@ -1756,6 +1756,25 @@ static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
>  	return 0;
>  }
>  
> +static netdev_features_t ibmveth_features_check(struct sk_buff *skb,
> +						struct net_device *dev,
> +						netdev_features_t features)
> +{
> +	/* Some physical adapters do not support segmentation offload with
> +	 * MSS < 224. Disable GSO for such packets to avoid adapter freeze.
> +	 */
> +	if (skb_is_gso(skb)) {

The commit message mentions this matches the ibmvnic hardware errata from
commit f10b09ef687f. That commit disabled GSO for both gso_size < 224 and
gso_segs == 1.

By omitting the check for skb_shinfo(skb)->gso_segs == 1, could a
single-segment GSO packet still cause the adapter to freeze?

> +		if (skb_shinfo(skb)->gso_size < IBMVETH_MIN_LSO_MSS) {
> +			netdev_warn_once(dev,
> +					 "MSS %u too small for LSO, disabling GSO\n",
> +					 skb_shinfo(skb)->gso_size);
> +			features &= ~NETIF_F_GSO_MASK;
> +		}
> +	}
> +
> +	return features;

When implementing a custom ndo_features_check callback, netif_skb_features()
will bypass the standard dflt_features_check() function. Does this mean
vlan_features_check() is no longer called for this driver?

If vlan_features_check() is skipped, multi-tagged (QinQ) VLAN packets
might retain GSO and protocol-specific checksum offloads that the hardware
parser cannot handle.

Should this return vlan_features_check(skb, features) instead?

> +}
-- 
pw-bot: cr

Re: [PATCH net v2] ibmveth: Disable GSO for packets with small MSS

[mingming cao]

On 4/18/26 10:54 AM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.

Thanks for the review and the questions!

> ---
> ibmveth: Disable GSO for packets with small MSS
>
> This commit adds an ndo_features_check callback to the ibmveth driver to
> disable segmentation offload when the MSS is less than 224 bytes. This
> prevents physical adapters in SEA configurations from freezing when given
> small MSS packets.
>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> [ ... ]
>
>> @@ -1756,6 +1756,25 @@ static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
>>   	return 0;
>>   }
>>   
>> +static netdev_features_t ibmveth_features_check(struct sk_buff *skb,
>> +						struct net_device *dev,
>> +						netdev_features_t features)
>> +{
>> +	/* Some physical adapters do not support segmentation offload with
>> +	 * MSS < 224. Disable GSO for such packets to avoid adapter freeze.
>> +	 */
>> +	if (skb_is_gso(skb)) {
> The commit message mentions this matches the ibmvnic hardware errata from
> commit f10b09ef687f. That commit disabled GSO for both gso_size < 224 and
> gso_segs == 1.
>
> By omitting the check for skb_shinfo(skb)->gso_segs == 1, could a
> single-segment GSO packet still cause the adapter to freeze?
Good question. Unlike ibmvnic, ibmveth does not need to check for 
single-segment GSO packets (gso_segs == 1).

In ibmvnic, the firmware processes all GSO packets, including 
single-segment ones, through the same LSO code path. This means even 
gso_segs == 1 packets can trigger the hardware errata.

In ibmveth, the PowerVM hypervisor intelligently bypasses the LSO path 
for single-segment packets. When gso_segs == 1, the hypervisor treats 
the packet as a regular (non-GSO) packet and transmits it directly 
without invoking the problematic hardware segmentation logic. Therefore, 
single-segment GSO packets never reach the code path that causes the freeze.

This architectural difference is why ibmvnic needs the gso_segs check 
but ibmveth does not.
>
>> +		if (skb_shinfo(skb)->gso_size < IBMVETH_MIN_LSO_MSS) {
>> +			netdev_warn_once(dev,
>> +					 "MSS %u too small for LSO, disabling GSO\n",
>> +					 skb_shinfo(skb)->gso_size);
>> +			features &= ~NETIF_F_GSO_MASK;
>> +		}
>> +	}
>> +
>> +	return features;
> When implementing a custom ndo_features_check callback, netif_skb_features()
> will bypass the standard dflt_features_check() function. Does this mean
> vlan_features_check() is no longer called for this driver?
>
> If vlan_features_check() is skipped, multi-tagged (QinQ) VLAN packets
> might retain GSO and protocol-specific checksum offloads that the hardware
> parser cannot handle.
>
> Should this return vlan_features_check(skb, features) instead?

You’re correct about the interaction with the default feature filtering. 
With a custom ndo_features_check(), the standard dflt_features_check() 
path is bypassed, so vlan_features_check() must be called explicitly to 
preserve existing VLAN/QinQ and checksum handling.

Thank you for catching this! I'll send v3 shortly with:
1. The vlan_features_check() call added
2. Updated commit message explaining why gso_segs check is not needed 
for ibmveth

Best regards,

Mingming


>> +}