On Fri, Jan 27, 2017 at 8:33 PM, Tom Herbert <t...@herbertland.com> wrote: > On Fri, Jan 27, 2017 at 10:19 AM, Saeed Mahameed > <sae...@dev.mellanox.co.il> wrote: >> On Fri, Jan 27, 2017 at 1:32 AM, Tom Herbert <t...@herbertland.com> wrote: >>> Add a configuration option (CONFIG_MLX5_CORE_ESWITCH) for controlling >>> whether the eswitch code is built. Change Kconfig and Makefile >>> accordingly. >>> >>> Signed-off-by: Tom Herbert <t...@herbertland.com> >>> --- >>> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 11 +++ >>> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 6 +- >>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 92 >>> +++++++++++++++++------ >>> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 39 +++++++--- >>> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 4 +- >>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 16 ++-- >>> drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 6 +- >>> 7 files changed, 125 insertions(+), 49 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig >>> b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig >>> index ddb4ca4..27aae58 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig >>> @@ -30,3 +30,14 @@ config MLX5_CORE_EN_DCB >>> This flag is depended on the kernel's DCB support. >>> >>> If unsure, set to Y >>> + >>> +config MLX5_CORE_EN_ESWITCH >>> + bool "Ethernet switch" >>> + default y >>> + depends on MLX5_CORE_EN >>> + ---help--- >>> + Say Y here if you want to use Ethernet switch (E-switch). E-Switch >>> + is the software entity that represents and manages ConnectX4 >>> + inter-HCA ethernet l2 switching. >>> + >>> + If unsure, set to Y >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile >>> b/drivers/net/ethernet/mellanox/mlx5/core/Makefile >>> index 9f43beb..17025d8 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile >>> @@ -5,9 +5,11 @@ mlx5_core-y := main.o cmd.o debugfs.o fw.o eq.o uar.o >>> pagealloc.o \ >>> mad.o transobj.o vport.o sriov.o fs_cmd.o fs_core.o \ >>> fs_counters.o rl.o lag.o dev.o >>> >>> -mlx5_core-$(CONFIG_MLX5_CORE_EN) += wq.o eswitch.o eswitch_offloads.o \ >>> +mlx5_core-$(CONFIG_MLX5_CORE_EN) += wq.o \ >>> en_main.o en_common.o en_fs.o en_ethtool.o en_tx.o \ >>> en_rx.o en_rx_am.o en_txrx.o en_clock.o vxlan.o \ >>> - en_tc.o en_arfs.o en_rep.o en_fs_ethtool.o en_selftest.o >>> + en_tc.o en_arfs.o en_fs_ethtool.o en_selftest.o >>> >>> mlx5_core-$(CONFIG_MLX5_CORE_EN_DCB) += en_dcbnl.o >>> + >>> +mlx5_core-$(CONFIG_MLX5_CORE_EN_ESWITCH) += eswitch.o eswitch_offloads.o >>> en_rep.o >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >>> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >>> index e829143..1db4d98 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >>> @@ -38,7 +38,9 @@ >>> #include <linux/bpf.h> >>> #include "en.h" >>> #include "en_tc.h" >>> +#ifdef CONFIG_MLX5_CORE_EN_ESWITCH >>> #include "eswitch.h" >>> +#endif >> >> Wouldn't it be cleaner if we left the main code (en_main.c) untouched >> and kept this >> #include "eswitch.h" and instead of filling the main flow code with >> #ifdefs, in eswitch.h >> we can create eswitch mock API functions when >> CONFIG_MLX5_CORE_EN_ESWITCH is not enabled ? the main flow will be >> clean from ifdefs and will complie with mock functions. >> >> we did this with accelerated RFS feature. look for "#ifndef >> CONFIG_RFS_ACCEL" in en.h >> > There are still occurrences of CONFIG_RFS_ACCEL in en_main.c and > main.c. For eswitch its a header problem, not everything related to > eswitch was extracted out of main though backend functions. There is a > lot of code that related to eswitch that is intertwined with the core > code. >
Interesting, i just did a quick look and it seems to me all eswitch logic in en_main.c can be kept untouched if we have the right mock functions, on the other hand it seems that there are a lot of eswitch functions to mock, i am not sure it is a good thing anymore, let's leave it as is for now.