[v4,4/6] net/ark: cleanup ark dynamic extension interface

Message ID 20210309160818.3553-4-ed.czeck@atomicrules.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v4,1/6] net/ark: update pkt director initial state |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ed Czeck March 9, 2021, 4:08 p.m. UTC
  - Rename extension functions with rte_pmd_ark prefix
- Move extension prototype to rte_pmd_ark.h
- Update local function documentation

Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
---
v3:
- split function rename from previous commit
v4:
- reorder patches renaming before adding
---
 drivers/net/ark/ark_ethdev.c  |  32 ++---
 drivers/net/ark/ark_ext.h     |  90 ------------
 drivers/net/ark/rte_pmd_ark.h | 251 +++++++++++++++++++++++++++++++++-
 3 files changed, 263 insertions(+), 110 deletions(-)
 delete mode 100644 drivers/net/ark/ark_ext.h
  

Comments

Ferruh Yigit March 9, 2021, 5:50 p.m. UTC | #1
On 3/9/2021 4:08 PM, Ed Czeck wrote:
> - Rename extension functions with rte_pmd_ark prefix
> - Move extension prototype to rte_pmd_ark.h
> - Update local function documentation
> 
> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> ---
> v3:
> - split function rename from previous commit
> v4:
> - reorder patches renaming before adding

<...>

> diff --git a/drivers/net/ark/rte_pmd_ark.h b/drivers/net/ark/rte_pmd_ark.h
> index 6f26d66b1..0f24a347d 100644
> --- a/drivers/net/ark/rte_pmd_ark.h
> +++ b/drivers/net/ark/rte_pmd_ark.h
> @@ -1,18 +1,24 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright (c) 2020 Atomic Rules LLC
> + * Copyright (c) 2020-2021 Atomic Rules LLC
>    */
>   
>   #ifndef RTE_PMD_ARK_H
>   #define RTE_PMD_ARK_H
>   
> +#include <rte_mbuf.h>
> +#include <rte_mbuf_dyn.h>
> +
> +#include <stdint.h>
> +struct rte_eth_dev;
> +struct rte_mbuf;
> +struct rte_ether_addr;
> +struct rte_eth_stats;
> +

Is the declaring structures instead of including header preferred intentionally?

I guess both works, and it may not differ for the PMD at all, but for the 
extension developer that is including this header, it may need to include 
required headers again, this header including all required headers and be a self 
contained may help extension code, up to you.

Plus just a detail but I guess mbuf struct is redundant for this patch, it can 
be added in next patch where mbufs header removed and hooks added.
  
Ed Czeck March 10, 2021, 3:11 p.m. UTC | #2
On Tue, Mar 9, 2021 at 12:50 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 3/9/2021 4:08 PM, Ed Czeck wrote:
> > - Rename extension functions with rte_pmd_ark prefix
> > - Move extension prototype to rte_pmd_ark.h
> > - Update local function documentation
> >
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> > ---
> > v3:
> > - split function rename from previous commit
> > v4:
> > - reorder patches renaming before adding
>
> <...>
>
> > diff --git a/drivers/net/ark/rte_pmd_ark.h b/drivers/net/ark/rte_pmd_ark.h
> > index 6f26d66b1..0f24a347d 100644
> > --- a/drivers/net/ark/rte_pmd_ark.h
> > +++ b/drivers/net/ark/rte_pmd_ark.h
> > @@ -1,18 +1,24 @@
> >   /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright (c) 2020 Atomic Rules LLC
> > + * Copyright (c) 2020-2021 Atomic Rules LLC
> >    */
> >
> >   #ifndef RTE_PMD_ARK_H
> >   #define RTE_PMD_ARK_H
> >
> > +#include <rte_mbuf.h>
> > +#include <rte_mbuf_dyn.h>
> > +
> > +#include <stdint.h>
> > +struct rte_eth_dev;
> > +struct rte_mbuf;
> > +struct rte_ether_addr;
> > +struct rte_eth_stats;
> > +
>
> Is the declaring structures instead of including header preferred intentionally?
>
> I guess both works, and it may not differ for the PMD at all, but for the
> extension developer that is including this header, it may need to include
> required headers again, this header including all required headers and be a self
> contained may help extension code, up to you.

My experience/style has been to use a declaration rather than a full include to
avoid include bloat and speed up compiles.  If the extension developer needs
the declaration in the header, they can include it as needed.

>
> Plus just a detail but I guess mbuf struct is redundant for this patch, it can
> be added in next patch where mbufs header removed and hooks added.

Agreed.  Would you like another patch?

Thanks
Ed.
  
Ferruh Yigit March 10, 2021, 4:29 p.m. UTC | #3
On 3/10/2021 3:11 PM, Ed Czeck wrote:
> On Tue, Mar 9, 2021 at 12:50 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 3/9/2021 4:08 PM, Ed Czeck wrote:
>>> - Rename extension functions with rte_pmd_ark prefix
>>> - Move extension prototype to rte_pmd_ark.h
>>> - Update local function documentation
>>>
>>> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
>>> ---
>>> v3:
>>> - split function rename from previous commit
>>> v4:
>>> - reorder patches renaming before adding
>>
>> <...>
>>
>>> diff --git a/drivers/net/ark/rte_pmd_ark.h b/drivers/net/ark/rte_pmd_ark.h
>>> index 6f26d66b1..0f24a347d 100644
>>> --- a/drivers/net/ark/rte_pmd_ark.h
>>> +++ b/drivers/net/ark/rte_pmd_ark.h
>>> @@ -1,18 +1,24 @@
>>>    /* SPDX-License-Identifier: BSD-3-Clause
>>> - * Copyright (c) 2020 Atomic Rules LLC
>>> + * Copyright (c) 2020-2021 Atomic Rules LLC
>>>     */
>>>
>>>    #ifndef RTE_PMD_ARK_H
>>>    #define RTE_PMD_ARK_H
>>>
>>> +#include <rte_mbuf.h>
>>> +#include <rte_mbuf_dyn.h>
>>> +
>>> +#include <stdint.h>
>>> +struct rte_eth_dev;
>>> +struct rte_mbuf;
>>> +struct rte_ether_addr;
>>> +struct rte_eth_stats;
>>> +
>>
>> Is the declaring structures instead of including header preferred intentionally?
>>
>> I guess both works, and it may not differ for the PMD at all, but for the
>> extension developer that is including this header, it may need to include
>> required headers again, this header including all required headers and be a self
>> contained may help extension code, up to you.
> 
> My experience/style has been to use a declaration rather than a full include to
> avoid include bloat and speed up compiles.  If the extension developer needs
> the declaration in the header, they can include it as needed.
> 

OK.

>>
>> Plus just a detail but I guess mbuf struct is redundant for this patch, it can
>> be added in next patch where mbufs header removed and hooks added.
> 
> Agreed.  Would you like another patch?
> 
> Thanks
> Ed.
>
  

Patch

diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 95546a891..5282534d3 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -193,58 +193,58 @@  check_for_ext(struct ark_adapter *ark)
 	/* Get the entry points */
 	ark->user_ext.dev_init =
 		(void *(*)(struct rte_eth_dev *, void *, int))
-		dlsym(ark->d_handle, "dev_init");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_init");
 	ARK_PMD_LOG(DEBUG, "device ext init pointer = %p\n",
 		      ark->user_ext.dev_init);
 	ark->user_ext.dev_get_port_count =
 		(int (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_get_port_count");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_get_port_count");
 	ark->user_ext.dev_uninit =
 		(void (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_uninit");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_uninit");
 	ark->user_ext.dev_configure =
 		(int (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_configure");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_configure");
 	ark->user_ext.dev_start =
 		(int (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_start");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_start");
 	ark->user_ext.dev_stop =
 		(void (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_stop");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_stop");
 	ark->user_ext.dev_close =
 		(void (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_close");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_close");
 	ark->user_ext.link_update =
 		(int (*)(struct rte_eth_dev *, int, void *))
-		dlsym(ark->d_handle, "link_update");
+		dlsym(ark->d_handle, "rte_pmd_ark_link_update");
 	ark->user_ext.dev_set_link_up =
 		(int (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_set_link_up");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_set_link_up");
 	ark->user_ext.dev_set_link_down =
 		(int (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "dev_set_link_down");
+		dlsym(ark->d_handle, "rte_pmd_ark_dev_set_link_down");
 	ark->user_ext.stats_get =
 		(int (*)(struct rte_eth_dev *, struct rte_eth_stats *,
 			  void *))
-		dlsym(ark->d_handle, "stats_get");
+		dlsym(ark->d_handle, "rte_pmd_ark_stats_get");
 	ark->user_ext.stats_reset =
 		(void (*)(struct rte_eth_dev *, void *))
-		dlsym(ark->d_handle, "stats_reset");
+		dlsym(ark->d_handle, "rte_pmd_ark_stats_reset");
 	ark->user_ext.mac_addr_add =
 		(void (*)(struct rte_eth_dev *, struct rte_ether_addr *,
 			uint32_t, uint32_t, void *))
-		dlsym(ark->d_handle, "mac_addr_add");
+		dlsym(ark->d_handle, "rte_pmd_ark_mac_addr_add");
 	ark->user_ext.mac_addr_remove =
 		(void (*)(struct rte_eth_dev *, uint32_t, void *))
-		dlsym(ark->d_handle, "mac_addr_remove");
+		dlsym(ark->d_handle, "rte_pmd_ark_mac_addr_remove");
 	ark->user_ext.mac_addr_set =
 		(void (*)(struct rte_eth_dev *, struct rte_ether_addr *,
 			  void *))
-		dlsym(ark->d_handle, "mac_addr_set");
+		dlsym(ark->d_handle, "rte_pmd_ark_mac_addr_set");
 	ark->user_ext.set_mtu =
 		(int (*)(struct rte_eth_dev *, uint16_t,
 			  void *))
-		dlsym(ark->d_handle, "set_mtu");
+		dlsym(ark->d_handle, "rte_pmd_ark_set_mtu");
 
 	return found;
 }
diff --git a/drivers/net/ark/ark_ext.h b/drivers/net/ark/ark_ext.h
deleted file mode 100644
index 821fb55bb..000000000
--- a/drivers/net/ark/ark_ext.h
+++ /dev/null
@@ -1,90 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2018 Atomic Rules LLC
- */
-
-#ifndef _ARK_EXT_H_
-#define _ARK_EXT_H_
-
-#include <ethdev_driver.h>
-
-/*
- * This is the template file for users who which to define a dynamic
- * extension to the Arkville PMD.   User's who create an extension
- * should include this file and define the necessary and desired
- * functions.
- * Only 1 function is required for an extension, dev_init(); all other
- * functions prototyped in this file are optional.
- */
-
-/*
- * Called post PMD init.
- * The implementation returns its private data that gets passed into
- * all other functions as user_data
- * The ARK extension implementation MUST implement this function
- */
-void *dev_init(struct rte_eth_dev *dev, void *a_bar, int port_id);
-
-/* Called during device shutdown */
-void dev_uninit(struct rte_eth_dev *dev, void *user_data);
-
-/* This call is optional and allows the
- * extension to specify the number of supported ports.
- */
-uint8_t dev_get_port_count(struct rte_eth_dev *dev,
-			   void *user_data);
-
-/*
- * The following functions are optional and are directly mapped
- * from the DPDK PMD ops structure.
- * Each function if implemented is called after the ARK PMD
- * implementation executes.
- */
-
-int dev_configure(struct rte_eth_dev *dev,
-		  void *user_data);
-
-int dev_start(struct rte_eth_dev *dev,
-	      void *user_data);
-
-void dev_stop(struct rte_eth_dev *dev,
-	      void *user_data);
-
-void dev_close(struct rte_eth_dev *dev,
-	       void *user_data);
-
-int link_update(struct rte_eth_dev *dev,
-		int wait_to_complete,
-		void *user_data);
-
-int dev_set_link_up(struct rte_eth_dev *dev,
-		    void *user_data);
-
-int dev_set_link_down(struct rte_eth_dev *dev,
-		      void *user_data);
-
-int stats_get(struct rte_eth_dev *dev,
-	       struct rte_eth_stats *stats,
-	       void *user_data);
-
-void stats_reset(struct rte_eth_dev *dev,
-		 void *user_data);
-
-void mac_addr_add(struct rte_eth_dev *dev,
-		  struct rte_ether_addr *macadr,
-		  uint32_t index,
-		  uint32_t pool,
-		  void *user_data);
-
-void mac_addr_remove(struct rte_eth_dev *dev,
-		     uint32_t index,
-		     void *user_data);
-
-void mac_addr_set(struct rte_eth_dev *dev,
-		  struct rte_ether_addr *mac_addr,
-		  void *user_data);
-
-int set_mtu(struct rte_eth_dev *dev,
-	    uint16_t size,
-	    void *user_data);
-
-#endif
diff --git a/drivers/net/ark/rte_pmd_ark.h b/drivers/net/ark/rte_pmd_ark.h
index 6f26d66b1..0f24a347d 100644
--- a/drivers/net/ark/rte_pmd_ark.h
+++ b/drivers/net/ark/rte_pmd_ark.h
@@ -1,18 +1,24 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2020 Atomic Rules LLC
+ * Copyright (c) 2020-2021 Atomic Rules LLC
  */
 
 #ifndef RTE_PMD_ARK_H
 #define RTE_PMD_ARK_H
 
+#include <rte_mbuf.h>
+#include <rte_mbuf_dyn.h>
+
+#include <stdint.h>
+struct rte_eth_dev;
+struct rte_mbuf;
+struct rte_ether_addr;
+struct rte_eth_stats;
+
 /**
  * @file
  * ARK driver-specific API
  */
 
-#include <rte_mbuf.h>
-#include <rte_mbuf_dyn.h>
-
 #ifndef RTE_PMD_ARK_TX_USERDATA_ENABLE
 #define RTE_PMD_ARK_TX_USERDATA_ENABLE 0
 #endif
@@ -122,4 +128,241 @@  rte_pmd_ark_mbuf_rx_userdata_set(struct rte_mbuf *mbuf,
 #endif
 }
 
+/* The following section lists function prototypes for Arkville's
+ * dynamic PMD extension. User's who create an extension
+ * must include this file and define the necessary and desired
+ * functions. Only 1 function is required for an extension,
+ * rte_pmd_ark_dev_init(); all other functions prototypes in this
+ * section are optional.
+ * See documentation for compiling and use of extensions.
+ */
+
+/**
+ * Extension prototype, required implementation if extensions are used.
+ * Called during device probe to initialize the user structure
+ * passed to other extension functions.  This is called once for each
+ * port of the device.
+ *
+ * @param dev
+ *   current device.
+ * @param a_bar
+ *   access to PCIe device bar (application bar) and hence access to
+ *   user's portion of FPGA.
+ * @param port_id
+ *   port identifier.
+ * @return user_data
+ *   which will be passed to other extension functions.
+ */
+void *rte_pmd_ark_dev_init(struct rte_eth_dev *dev, void *a_bar, int port_id);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during device uninit.
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ */
+void rte_pmd_ark_dev_uninit(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during device probe to change the port count from 1.
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+uint8_t dev_get_port_count(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_configure().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_dev_configure(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_start().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_dev_start(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during  rte_eth_dev_stop().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_dev_stop(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_close().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_dev_close(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during link_update status event.
+ *
+ * @param dev
+ *   current device.
+ * @param wait_to_complete
+ *    argument from update event.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_link_update(struct rte_eth_dev *dev,
+			    int wait_to_complete,
+			    void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_set_link_up().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_dev_set_link_up(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_set_link_down().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_dev_set_link_down(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_stats_get(); allows updates to the stats
+ * struct in addition Ark's PMD operations.
+ *
+ * @param dev
+ *   current device.
+ * @param stats
+ *   statistics struct already populated by Ark PMD.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_stats_get(struct rte_eth_dev *dev,
+			  struct rte_eth_stats *stats,
+			  void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_stats_reset().
+ *
+ * @param dev
+ *   current device.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_stats_reset(struct rte_eth_dev *dev, void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_mac_addr_add().
+ *
+ * @param dev
+ *   current device.
+ * @param macaddr
+ *   The MAC address to add
+ * @param index
+ *   The index into the MAC address array.
+ * @param pool
+ *   VMDq pool index from caller
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_mac_addr_add(struct rte_eth_dev *dev,
+			      struct rte_ether_addr *macaddr,
+			      uint32_t index,
+			      uint32_t pool,
+			      void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_mac_addr_remove().
+ *
+ * @param dev
+ *   current device.
+ * @param index
+ *   The index into the MAC address array.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_mac_addr_remove(struct rte_eth_dev *dev,
+				 uint32_t index,
+				 void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_default_mac_addr_set().
+ *
+ * @param dev
+ *   current device.
+ * @param mac_addr
+ *   The new default MAC address.
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+void rte_pmd_ark_mac_addr_set(struct rte_eth_dev *dev,
+			      struct rte_ether_addr *mac_addr,
+			      void *user_data);
+
+/**
+ * Extension prototype, optional implementation.
+ * Called during rte_eth_dev_set_mtu().
+ *
+ * @param dev
+ *   current device.
+ * @param size
+ *   The MTU to be applied
+ * @param user_data
+ *   user argument from dev_init() call.
+ * @return (0) if successful.
+ */
+int rte_pmd_ark_set_mtu(struct rte_eth_dev *dev,
+			uint16_t size,
+			void *user_data);
+
 #endif /* RTE_PMD_ARK_H */