[PATCH net v2 0/1] ibmvnic: Fix for send scrq direct

[PATCH net v2 0/1] ibmvnic: Fix for send scrq direct

[Nick Child]

This is a v2 of a patchset (now just patch) which addresses a
bug in a new feature which is causing major link UP issues with
certain physical cards.

For a full summary of the issue:
  1. During vnic initialization we get the following values from vnic
     server regarding "Transmit / Receive Descriptor Requirement" (see
      PAPR Table 584. CAPABILITIES Commands):
    - LSO Tx frame = 0x0F , header offsets + L2, L3, L4 headers required
    - CSO Tx frame = 0x0C , header offsets + L2 header required
    - standard frame = 0x0C , header offsets + L2 header required
  2. Assume we are dealing with only "standard frames" from now on (no
     CSO, no LSO)
  3. When using 100G backing device, we don't hand vnic server any header
     information and TX is successful
  4. When using 25G backing device, we don't hand vnic server any header
    information and TX fails and we get "Adapter Error" transport events.
The obvious issue here is that vnic client should be respecting the 0X0C
header requirement for standard frames.  But 100G cards will also give
0x0C despite the fact that we know TX works if we ignore it. That being
said, we still must respect values given from the managing server. Will
need to work with them going forward to hopefully get 100G cards to
return 0x00 for this bitstring so the performance gains of using
send_subcrq_direct can be continued.

Thanks Simon for review on v1 [1]. Much appreciated.

Changes in v2:
 - drop the statistics tracking patch (send to net-next)
 - Fix formatting and commit of Fixes tag


Nick Child (1):
  ibmvnic: Inspect header requirements before using scrq direct

 drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.43.5

[PATCH net v2 1/1] ibmvnic: Inspect header requirements before using scrq direct

[Nick Child]

Previously, the TX header requirement for standard frames was ignored.
This requirement is a bitstring sent from the VIOS which maps to the
type of header information needed during TX. If no header information,
is needed then send subcrq direct can be used (which can be more
performant).

This bitstring was previously ignored for standard packets (AKA non LSO,
non CSO) due to the belief that the bitstring was over-cautionary. It
turns out that there are some configurations where the backing device
does need header information for transmission of standard packets. If
the information is not supplied then this causes continuous "Adapter
error" transport events. Therefore, this bitstring should be respected
and observed before considering the use of send subcrq direct.

Fixes: 74839f7a8268 ("ibmvnic: Introduce send sub-crq direct")
Signed-off-by: Nick Child <nnac123@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 53b309ddc63b..cca2ed6ad289 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2473,9 +2473,11 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 	/* if we are going to send_subcrq_direct this then we need to
 	 * update the checksum before copying the data into ltb. Essentially
 	 * these packets force disable CSO so that we can guarantee that
-	 * FW does not need header info and we can send direct.
+	 * FW does not need header info and we can send direct. Also, vnic
+	 * server must be able to xmit standard packets without header data
 	 */
-	if (!skb_is_gso(skb) && !ind_bufp->index && !netdev_xmit_more()) {
+	if (*hdrs == 0 && !skb_is_gso(skb) &&
+	    !ind_bufp->index && !netdev_xmit_more()) {
 		use_scrq_send_direct = true;
 		if (skb->ip_summed == CHECKSUM_PARTIAL &&
 		    skb_checksum_help(skb))
-- 
2.43.5

Re: [PATCH net v2 1/1] ibmvnic: Inspect header requirements before using scrq direct

[Simon Horman]

On Tue, Oct 01, 2024 at 11:32:00AM -0500, Nick Child wrote:
> Previously, the TX header requirement for standard frames was ignored.
> This requirement is a bitstring sent from the VIOS which maps to the
> type of header information needed during TX. If no header information,
> is needed then send subcrq direct can be used (which can be more
> performant).
> 
> This bitstring was previously ignored for standard packets (AKA non LSO,
> non CSO) due to the belief that the bitstring was over-cautionary. It
> turns out that there are some configurations where the backing device
> does need header information for transmission of standard packets. If
> the information is not supplied then this causes continuous "Adapter
> error" transport events. Therefore, this bitstring should be respected
> and observed before considering the use of send subcrq direct.
> 
> Fixes: 74839f7a8268 ("ibmvnic: Introduce send sub-crq direct")
> Signed-off-by: Nick Child <nnac123@linux.ibm.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2 0/1] ibmvnic: Fix for send scrq direct

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  1 Oct 2024 11:31:59 -0500 you wrote:
> This is a v2 of a patchset (now just patch) which addresses a
> bug in a new feature which is causing major link UP issues with
> certain physical cards.
> 
> For a full summary of the issue:
>   1. During vnic initialization we get the following values from vnic
>      server regarding "Transmit / Receive Descriptor Requirement" (see
>       PAPR Table 584. CAPABILITIES Commands):
>     - LSO Tx frame = 0x0F , header offsets + L2, L3, L4 headers required
>     - CSO Tx frame = 0x0C , header offsets + L2 header required
>     - standard frame = 0x0C , header offsets + L2 header required
>   2. Assume we are dealing with only "standard frames" from now on (no
>      CSO, no LSO)
>   3. When using 100G backing device, we don't hand vnic server any header
>      information and TX is successful
>   4. When using 25G backing device, we don't hand vnic server any header
>     information and TX fails and we get "Adapter Error" transport events.
> The obvious issue here is that vnic client should be respecting the 0X0C
> header requirement for standard frames.  But 100G cards will also give
> 0x0C despite the fact that we know TX works if we ignore it. That being
> said, we still must respect values given from the managing server. Will
> need to work with them going forward to hopefully get 100G cards to
> return 0x00 for this bitstring so the performance gains of using
> send_subcrq_direct can be continued.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/1] ibmvnic: Inspect header requirements before using scrq direct
    https://git.kernel.org/netdev/net/c/de390657b5d6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html