Skip to content

Commit

Permalink
ice: remove circular header dependencies on ice.h
Browse files Browse the repository at this point in the history
Several headers in the ice driver include ice.h even though they are
themselves included by that header. The most notable of these is
ice_common.h, but several other headers also do this.

Such a recursive inclusion is problematic as it forces headers to be
included in a strict order, otherwise compilation errors can result. The
circular inclusions do not trigger an endless loop due to standard
header inclusion guards, however other errors can occur.

For example, ice_flow.h defines ice_rss_hash_cfg, which is used by
ice_sriov.h as part of the definition of ice_vf_hash_ip_ctx.

ice_flow.h includes ice_acl.h, which includes ice_common.h, and which
finally includes ice.h. Since ice.h itself includes ice_sriov.h, this
creates a circular dependency.

The definition in ice_sriov.h requires things from ice_flow.h, but
ice_flow.h itself will lead to trying to load ice_sriov.h as part of its
process for expanding ice.h. The current code avoids this issue by
having an implicit dependency without the include of ice_flow.h.

If we were to fix that so that ice_sriov.h explicitly depends on
ice_flow.h the following pattern would occur:

  ice_flow.h -> ice_acl.h -> ice_common.h -> ice.h -> ice_sriov.h

At this point, during the expansion of, the header guard for ice_flow.h
is already set, so when ice_sriov.h attempts to load the ice_flow.h
header it is skipped. Then, we go on to begin including the rest of
ice_sriov.h, including structure definitions which depend on
ice_rss_hash_cfg. This produces a compiler warning because
ice_rss_hash_cfg hasn't yet been included. Remember, we're just at the
start of ice_flow.h!

If the order of headers is incorrect (ice_flow.h is not implicitly
loaded first in all files which include ice_sriov.h) then we get the
same failure.

Removing this recursive inclusion requires fixing a few cases where some
headers depended on the header inclusions from ice.h. In addition, a few
other changes are also required.

Most notably, ice_hw_to_dev is implemented as a macro in ice_osdep.h,
which is the likely reason that ice_common.h includes ice.h at all. This
macro implementation requires the full definition of ice_pf in order to
properly compile.

Fix this by moving it to a function declared in ice_main.c, so that we
do not require all files to depend on the layout of the ice_pf
structure.

Note that this change only fixes circular dependencies, but it does not
fully resolve all implicit dependencies where one header may depend on
the inclusion of another. I tried to fix as many of the implicit
dependencies as I noticed, but fixing them all requires a somewhat
tedious analysis of each header and attempting to compile it separately.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
Jacob Keller authored and Tony Nguyen committed Mar 15, 2022
1 parent 0deb0bf commit 649c87c
Show file tree
Hide file tree
Showing 14 changed files with 35 additions and 11 deletions.
3 changes: 0 additions & 3 deletions drivers/net/ethernet/intel/ice/ice.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@
#include <net/udp_tunnel.h>
#include <net/vxlan.h>
#include <net/gtp.h>
#if IS_ENABLED(CONFIG_DCB)
#include <scsi/iscsi_proto.h>
#endif /* CONFIG_DCB */
#include "ice_devids.h"
#include "ice_type.h"
#include "ice_txrx.h"
Expand Down
3 changes: 3 additions & 0 deletions drivers/net/ethernet/intel/ice/ice_arfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

#ifndef _ICE_ARFS_H_
#define _ICE_ARFS_H_

#include "ice_fdir.h"

enum ice_arfs_fltr_state {
ICE_ARFS_INACTIVE,
ICE_ARFS_ACTIVE,
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/intel/ice/ice_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

#include <linux/bitfield.h>

#include "ice.h"
#include "ice_type.h"
#include "ice_nvm.h"
#include "ice_flex_pipe.h"
#include "ice_switch.h"
#include <linux/avf/virtchnl.h>
#include "ice_switch.h"
#include "ice_fdir.h"

#define ICE_SQ_SEND_DELAY_TIME_MS 10
#define ICE_SQ_SEND_MAX_EXECUTE 3
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ethernet/intel/ice/ice_dcb.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define _ICE_DCB_H_

#include "ice_type.h"
#include <scsi/iscsi_proto.h>

#define ICE_DCBX_STATUS_NOT_STARTED 0
#define ICE_DCBX_STATUS_IN_PROGRESS 1
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ethernet/intel/ice/ice_flex_pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "ice_common.h"
#include "ice_flex_pipe.h"
#include "ice_flow.h"
#include "ice.h"

/* For supporting double VLAN mode, it is necessary to enable or disable certain
* boost tcam entries. The metadata labels names that match the following
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ethernet/intel/ice/ice_flow.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "ice_common.h"
#include "ice_flow.h"
#include <net/gre.h>

/* Describe properties of a protocol header field */
struct ice_flow_field_info {
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/ethernet/intel/ice/ice_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#ifndef _ICE_FLOW_H_
#define _ICE_FLOW_H_

#include "ice_flex_type.h"

#define ICE_FLOW_ENTRY_HANDLE_INVAL 0
#define ICE_FLOW_FLD_OFF_INVAL 0xffff

Expand Down
1 change: 0 additions & 1 deletion drivers/net/ethernet/intel/ice/ice_idc_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#define _ICE_IDC_INT_H_

#include <linux/net/intel/iidc.h>
#include "ice.h"

struct ice_pf;

Expand Down
15 changes: 15 additions & 0 deletions drivers/net/ethernet/intel/ice/ice_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,21 @@ static DEFINE_IDA(ice_aux_ida);
DEFINE_STATIC_KEY_FALSE(ice_xdp_locking_key);
EXPORT_SYMBOL(ice_xdp_locking_key);

/**
* ice_hw_to_dev - Get device pointer from the hardware structure
* @hw: pointer to the device HW structure
*
* Used to access the device pointer from compilation units which can't easily
* include the definition of struct ice_pf without leading to circular header
* dependencies.
*/
struct device *ice_hw_to_dev(struct ice_hw *hw)
{
struct ice_pf *pf = container_of(hw, struct ice_pf, hw);

return &pf->pdev->dev;
}

static struct workqueue_struct *ice_wq;
static const struct net_device_ops ice_netdev_safe_mode_ops;
static const struct net_device_ops ice_netdev_ops;
Expand Down
11 changes: 9 additions & 2 deletions drivers/net/ethernet/intel/ice/ice_osdep.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
#define _ICE_OSDEP_H_

#include <linux/types.h>
#include <linux/ctype.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/bitops.h>
#include <linux/ethtool.h>
#include <linux/etherdevice.h>
#include <linux/if_ether.h>
#include <linux/pci_ids.h>
#ifndef CONFIG_64BIT
#include <linux/io-64-nonatomic-lo-hi.h>
#endif
Expand All @@ -25,8 +32,8 @@ struct ice_dma_mem {
size_t size;
};

#define ice_hw_to_dev(ptr) \
(&(container_of((ptr), struct ice_pf, hw))->pdev->dev)
struct ice_hw;
struct device *ice_hw_to_dev(struct ice_hw *hw);

#ifdef CONFIG_DYNAMIC_DEBUG
#define ice_debug(hw, type, fmt, args...) \
Expand Down
1 change: 0 additions & 1 deletion drivers/net/ethernet/intel/ice/ice_repr.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#define _ICE_REPR_H_

#include <net/dst_metadata.h>
#include "ice.h"

struct ice_repr {
struct ice_vsi *src_vsi;
Expand Down
1 change: 0 additions & 1 deletion drivers/net/ethernet/intel/ice/ice_sriov.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#ifndef _ICE_SRIOV_H_
#define _ICE_SRIOV_H_
#include "ice.h"
#include "ice_virtchnl_fdir.h"
#include "ice_vsi_vlan_ops.h"

Expand Down
1 change: 1 addition & 0 deletions drivers/net/ethernet/intel/ice/ice_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#define ICE_CHNL_MAX_TC 16

#include "ice_hw_autogen.h"
#include "ice_devids.h"
#include "ice_osdep.h"
#include "ice_controlq.h"
#include "ice_lan_tx_rx.h"
Expand Down
1 change: 0 additions & 1 deletion drivers/net/ethernet/intel/ice/ice_xsk.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#ifndef _ICE_XSK_H_
#define _ICE_XSK_H_
#include "ice_txrx.h"
#include "ice.h"

#define PKTS_PER_BATCH 8

Expand Down

0 comments on commit 649c87c

Please sign in to comment.