netdev - Re: [PATCH net-next] sfc: do not try to call tc functions when CONFIG_SFC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZIl608QjU0HoX5l7@gmail.com>
Date: Wed, 14 Jun 2023 09:31:15 +0100
From: Martin Habets <habetsm.xilinx@...il.com>
To: edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
	pabeni@...hat.com, edumazet@...gle.com,
	Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org,
	oe-kbuild-all@...ts.linux.dev, simon.horman@...igine.com,
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH net-next] sfc: do not try to call tc functions when
 CONFIG_SFC_SRIOV=n

On Mon, Jun 12, 2023 at 09:54:28PM +0100, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Functions efx_tc_netdev_event and efx_tc_netevent_event do not exist
>  in that case as object files tc_bindings.o and tc_encap_actions.o
>  are not built, so the calls to them from ef100_netdev_event and
>  ef100_netevent_event cause link errors.
> Guard the relevant part of ef100_netdev_event with #ifdef, as well as
>  the entire function ef100_netevent_event and the code that registers
>  and unregisters the netevent notifier.
> Also guard the includes of tc_bindings.h and tc_encap_actions.h into
>  ef100_netdev.c, as the symbols from these headers are only available
>  when the corresponding object files are built.
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202306102026.ISK5JfUQ-lkp@intel.com/
> Fixes: 7e5e7d800011 ("sfc: neighbour lookup for TC encap action offload")
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
> ---
>  drivers/net/ethernet/sfc/ef100_netdev.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index 7f7d560cb2b4..3bb0442de7ea 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -23,8 +23,10 @@
>  #include "mcdi_filters.h"
>  #include "rx_common.h"
>  #include "ef100_sriov.h"
> +#ifdef CONFIG_SFC_SRIOV
>  #include "tc_bindings.h"
>  #include "tc_encap_actions.h"
> +#endif

Don't do this, it is old-style coding.
Put a static inline function definition inside the .h files
for !CONFIG_SFC_SRIOV. Just for the APIs you need.

>  #include "efx_devlink.h"
>  
>  static void ef100_update_name(struct efx_nic *efx)
> @@ -301,22 +303,27 @@ int ef100_netdev_event(struct notifier_block *this,
>  {
>  	struct efx_nic *efx = container_of(this, struct efx_nic, netdev_notifier);
>  	struct net_device *net_dev = netdev_notifier_info_to_dev(ptr);
> +#ifdef CONFIG_SFC_SRIOV
>  	struct ef100_nic_data *nic_data = efx->nic_data;
>  	int err;
> +#endif
>  
>  	if (efx->net_dev == net_dev &&
>  	    (event == NETDEV_CHANGENAME || event == NETDEV_REGISTER))
>  		ef100_update_name(efx);
>  
> +#ifdef CONFIG_SFC_SRIOV
>  	if (!nic_data->grp_mae)

Use IS_ENABLED(CONFIG_SFC_SRIOV) here, like was done in commit a59f832a71c9.

Martin

>  		return NOTIFY_DONE;
>  	err = efx_tc_netdev_event(efx, event, net_dev);
>  	if (err & NOTIFY_STOP_MASK)
>  		return err;
> +#endif
>  
>  	return NOTIFY_DONE;
>  }
>  
> +#ifdef CONFIG_SFC_SRIOV
>  static int ef100_netevent_event(struct notifier_block *this,
>  				unsigned long event, void *ptr)
>  {
> @@ -329,9 +336,9 @@ static int ef100_netevent_event(struct notifier_block *this,
>  	err = efx_tc_netevent_event(efx, event, ptr);
>  	if (err & NOTIFY_STOP_MASK)
>  		return err;
> -
>  	return NOTIFY_DONE;
>  };
> +#endif
>  
>  static int ef100_register_netdev(struct efx_nic *efx)
>  {
> @@ -392,8 +399,8 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
>  	rtnl_unlock();
>  
>  	unregister_netdevice_notifier(&efx->netdev_notifier);
> -	unregister_netevent_notifier(&efx->netevent_notifier);
>  #if defined(CONFIG_SFC_SRIOV)
> +	unregister_netevent_notifier(&efx->netevent_notifier);
>  	if (!efx->type->is_vf)
>  		efx_ef100_pci_sriov_disable(efx, true);
>  #endif
> @@ -513,6 +520,7 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  		goto fail;
>  	}
>  
> +#ifdef CONFIG_SFC_SRIOV
>  	efx->netevent_notifier.notifier_call = ef100_netevent_event;
>  	rc = register_netevent_notifier(&efx->netevent_notifier);
>  	if (rc) {
> @@ -520,6 +528,7 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  			  "Failed to register netevent notifier, rc=%d\n", rc);
>  		goto fail;
>  	}
> +#endif
>  
>  	efx_probe_devlink_unlock(efx);
>  	return rc;