Re: [PATCH net] sfc: fix NULL dereferences in ef100_process_design_param()

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Tue, Apr 01, 2025 at 11:54:39PM +0100, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Since cited commit, ef100_probe_main() and hence also
>  ef100_check_design_params() run before efx->net_dev is created;
>  consequently, we cannot netif_set_tso_max_size() or _segs() at this
>  point.
> Move those netif calls to ef100_probe_netdev(), and also replace
>  netif_err within the design params code with pci_err.
> 
> Reported-by: Kyungwook Boo <bookyungwook@gmail.com>
> Fixes: 98ff4c7c8ac7 ("sfc: Separate netdev probe/remove from PCI probe/remove")
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  drivers/net/ethernet/sfc/ef100_netdev.c |  6 ++--
>  drivers/net/ethernet/sfc/ef100_nic.c    | 47 +++++++++++--------------
>  2 files changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index d941f073f1eb..3a06e3b1bd6b 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -450,8 +450,9 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  	net_dev->hw_enc_features |= efx->type->offload_features;
>  	net_dev->vlan_features |= NETIF_F_HW_CSUM | NETIF_F_SG |
>  				  NETIF_F_HIGHDMA | NETIF_F_ALL_TSO;
> -	netif_set_tso_max_segs(net_dev,
> -			       ESE_EF100_DP_GZ_TSO_MAX_HDR_NUM_SEGS_DEFAULT);
> +	nic_data = efx->nic_data;
> +	netif_set_tso_max_size(efx->net_dev, nic_data->tso_max_payload_len);
> +	netif_set_tso_max_segs(efx->net_dev, nic_data->tso_max_payload_num_segs);

Is it fine to drop default value for max segs? Previously if somehow
this value wasn't read from HW it was set to default, now it will be 0.

At the beggining of ef100_probe_main() default values for nic_data are
set. Maybe it is worth to set also this default for max segs?

>  
>  	rc = efx_ef100_init_datapath_caps(efx);
>  	if (rc < 0)
> @@ -477,7 +478,6 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  	/* Don't fail init if RSS setup doesn't work. */
>  	efx_mcdi_push_default_indir_table(efx, efx->n_rx_channels);
>  
> -	nic_data = efx->nic_data;
>  	rc = ef100_get_mac_address(efx, net_dev->perm_addr, CLIENT_HANDLE_SELF,
>  				   efx->type->is_vf);
>  	if (rc)

[...]