diff mbox series

[v1,7/8] net/mlx5: add mlx5 header file specific to Linux

Message ID 20200603150602.4686-8-ophirmu@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series mlx5 PMD multi OS support |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Ophir Munk June 3, 2020, 3:06 p.m. UTC 
  File drivers/net/linux/mlx5_os.h is added. It includes specific
Linux definitions such as PCI driver flags, link state changes
interrupts, link removal interrupts, etc.

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/Makefile        |  1 +
 drivers/net/mlx5/linux/mlx5_os.h | 18 ++++++++++++++++++
 drivers/net/mlx5/mlx5.c          |  3 +--
 drivers/net/mlx5/mlx5.h          |  5 +++--
 4 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/mlx5/linux/mlx5_os.h

Comments

Ferruh Yigit June 8, 2020, 11:31 a.m. UTC | #1 
On 6/3/2020 4:06 PM, Ophir Munk wrote:
> File drivers/net/linux/mlx5_os.h is added. It includes specific
> Linux definitions such as PCI driver flags, link state changes
> interrupts, link removal interrupts, etc.
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>

<...>

> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index f5d9aad..eca4472 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -41,6 +41,7 @@
>  
>  #include "mlx5_defs.h"
>  #include "mlx5_utils.h"
> +#include "mlx5_os.h"

Assuming that you will have multiple "mlx5_os.h", one for each OS, like
"linux/mlx5_os.h" & "windows/mlx5_os.h", doesn't it make sense to include it as
"#include linux/mlx5_os.h", and remove relevant "-I" from CFLAGS in makefile?
Ophir Munk June 9, 2020, 8:44 a.m. UTC | #2 
Hi,
Please find comments inline.

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, June 8, 2020 2:32 PM
> To: Ophir Munk <ophirmu@mellanox.com>; dev@dpdk.org; Matan Azrad
> <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v1 7/8] net/mlx5: add mlx5 header file
> specific to Linux
> 
> On 6/3/2020 4:06 PM, Ophir Munk wrote:
> > File drivers/net/linux/mlx5_os.h is added. It includes specific Linux
> > definitions such as PCI driver flags, link state changes interrupts,
> > link removal interrupts, etc.
> >
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> > Acked-by: Matan Azrad <matan@mellanox.com>
> 
> <...>
> 
> > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > f5d9aad..eca4472 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -41,6 +41,7 @@
> >
> >  #include "mlx5_defs.h"
> >  #include "mlx5_utils.h"
> > +#include "mlx5_os.h"
> 
> Assuming that you will have multiple "mlx5_os.h", one for each OS, like
> "linux/mlx5_os.h" & "windows/mlx5_os.h", doesn't it make sense to include
> it as "#include linux/mlx5_os.h", and remove relevant "-I" from CFLAGS in
> makefile?

IMO it doesn't make sense. 
mlx5.h is a shared file that will be compiled under Windows as well. 
It wouldn't be possible if I used #include linux/mlx5_os.h
Ferruh Yigit June 9, 2020, 11:48 a.m. UTC | #3 
On 6/9/2020 9:44 AM, Ophir Munk wrote:
> Hi,
> Please find comments inline.
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, June 8, 2020 2:32 PM
>> To: Ophir Munk <ophirmu@mellanox.com>; dev@dpdk.org; Matan Azrad
>> <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH v1 7/8] net/mlx5: add mlx5 header file
>> specific to Linux
>>
>> On 6/3/2020 4:06 PM, Ophir Munk wrote:
>>> File drivers/net/linux/mlx5_os.h is added. It includes specific Linux
>>> definitions such as PCI driver flags, link state changes interrupts,
>>> link removal interrupts, etc.
>>>
>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>>> Acked-by: Matan Azrad <matan@mellanox.com>
>>
>> <...>
>>
>>> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
>>> f5d9aad..eca4472 100644
>>> --- a/drivers/net/mlx5/mlx5.h
>>> +++ b/drivers/net/mlx5/mlx5.h
>>> @@ -41,6 +41,7 @@
>>>
>>>  #include "mlx5_defs.h"
>>>  #include "mlx5_utils.h"
>>> +#include "mlx5_os.h"
>>
>> Assuming that you will have multiple "mlx5_os.h", one for each OS, like
>> "linux/mlx5_os.h" & "windows/mlx5_os.h", doesn't it make sense to include
>> it as "#include linux/mlx5_os.h", and remove relevant "-I" from CFLAGS in
>> makefile?
> 
> IMO it doesn't make sense. 
> mlx5.h is a shared file that will be compiled under Windows as well. 
> It wouldn't be possible if I used #include linux/mlx5_os.h
> 

It is possible with an #ifdef around include. (#ifdef Linux)

But if you keep as #include "mlx5_os.h" and have this header for multiple OS,
than you will have to have the ifdef in the build files.

Right now you are not doing both since there is only one platform support, I am
OK to proceed and postpone the second platform support until it is ready.
Ophir Munk June 9, 2020, 2:49 p.m. UTC | #4 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, June 9, 2020 2:49 PM
> To: Ophir Munk <ophirmu@mellanox.com>; dev@dpdk.org; Matan Azrad
> <matan@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v1 7/8] net/mlx5: add mlx5 header file
> specific to Linux
> <...>
> >> On 6/3/2020 4:06 PM, Ophir Munk wrote:
> >>> File drivers/net/linux/mlx5_os.h is added. It includes specific
> >>> Linux definitions such as PCI driver flags, link state changes
> >>> interrupts, link removal interrupts, etc.
> >>>
> >>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> >>> Acked-by: Matan Azrad <matan@mellanox.com>
> >>
> >> <...>
> >>
> >>> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> >>> f5d9aad..eca4472 100644
> >>> --- a/drivers/net/mlx5/mlx5.h
> >>> +++ b/drivers/net/mlx5/mlx5.h
> >>> @@ -41,6 +41,7 @@
> >>>
> >>>  #include "mlx5_defs.h"
> >>>  #include "mlx5_utils.h"
> >>> +#include "mlx5_os.h"
> >>
> >> Assuming that you will have multiple "mlx5_os.h", one for each OS,
> >> like "linux/mlx5_os.h" & "windows/mlx5_os.h", doesn't it make sense
> >> to include it as "#include linux/mlx5_os.h", and remove relevant "-I"
> >> from CFLAGS in makefile?
> >
> > IMO it doesn't make sense.
> > mlx5.h is a shared file that will be compiled under Windows as well.
> > It wouldn't be possible if I used #include linux/mlx5_os.h
> >
> 
> It is possible with an #ifdef around include. (#ifdef Linux)
> 
> But if you keep as #include "mlx5_os.h" and have this header for multiple OS,
> than you will have to have the ifdef in the build files.
> 
> Right now you are not doing both since there is only one platform support, I
> am OK to proceed and postpone the second platform support until it is ready.

Please note that Windows dpdk will only be built with the meson build system (no Makefile usage under Windows).
diff mbox series

Patch

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index 115b66c..41ab73e 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -41,6 +41,7 @@  CFLAGS += -g
 CFLAGS += -I$(RTE_SDK)/drivers/common/mlx5
 CFLAGS += -I$(RTE_SDK)/drivers/common/mlx5/linux
 CFLAGS += -I$(RTE_SDK)/drivers/net/mlx5
+CFLAGS += -I$(RTE_SDK)/drivers/net/mlx5/linux
 CFLAGS += -I$(BUILDDIR)/drivers/common/mlx5
 CFLAGS += -D_BSD_SOURCE
 CFLAGS += -D_DEFAULT_SOURCE
diff --git a/drivers/net/mlx5/linux/mlx5_os.h b/drivers/net/mlx5/linux/mlx5_os.h
new file mode 100644
index 0000000..f310f17
--- /dev/null
+++ b/drivers/net/mlx5/linux/mlx5_os.h
@@ -0,0 +1,18 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2015 6WIND S.A.
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef RTE_PMD_MLX5_OS_H_
+#define RTE_PMD_MLX5_OS_H_
+
+/* verb enumerations translations to local enums. */
+enum {
+	DEV_SYSFS_NAME_MAX = IBV_SYSFS_NAME_MAX,
+	DEV_SYSFS_PATH_MAX = IBV_SYSFS_PATH_MAX
+};
+
+#define PCI_DRV_FLAGS  (RTE_PCI_DRV_INTR_LSC | \
+			RTE_PCI_DRV_INTR_RMV | \
+			RTE_PCI_DRV_PROBE_AGAIN)
+#endif /* RTE_PMD_MLX5_OS_H_ */
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index f62ad12..16ab8b0 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2114,8 +2114,7 @@  struct rte_pci_driver mlx5_driver = {
 	.remove = mlx5_pci_remove,
 	.dma_map = mlx5_dma_map,
 	.dma_unmap = mlx5_dma_unmap,
-	.drv_flags = RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_INTR_RMV |
-		     RTE_PCI_DRV_PROBE_AGAIN,
+	.drv_flags = PCI_DRV_FLAGS,
 };
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index f5d9aad..eca4472 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -41,6 +41,7 @@ 
 
 #include "mlx5_defs.h"
 #include "mlx5_utils.h"
+#include "mlx5_os.h"
 #include "mlx5_autoconf.h"
 
 enum mlx5_ipool_index {
@@ -536,8 +537,8 @@  struct mlx5_dev_ctx_shared {
 	void *pd; /* Protection Domain. */
 	uint32_t pdn; /* Protection Domain number. */
 	uint32_t tdn; /* Transport Domain number. */
-	char ibdev_name[IBV_SYSFS_NAME_MAX]; /* IB device name. */
-	char ibdev_path[IBV_SYSFS_PATH_MAX]; /* IB device path for secondary */
+	char ibdev_name[DEV_SYSFS_NAME_MAX]; /* SYSFS dev name. */
+	char ibdev_path[DEV_SYSFS_PATH_MAX]; /* SYSFS dev path for secondary */
 	struct mlx5_dev_attr device_attr; /* Device properties. */
 	LIST_ENTRY(mlx5_dev_ctx_shared) mem_event_cb;
 	/**< Called by memory event callback. */