From: Jiri Pirko <jiri@resnulli.us>
To: alejandro.lucero-palau@amd.com
Cc: netdev@vger.kernel.org, linux-net-drivers@amd.com,
davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
edumazet@google.com, habetsm.xilinx@gmail.com,
ecree.xilinx@gmail.com, linux-doc@vger.kernel.org,
corbet@lwn.net, jiri@nvidia.com
Subject: Re: [PATCH v4 net-next 5/8] sfc: add devlink port support for ef100
Date: Tue, 31 Jan 2023 17:12:11 +0100 [thread overview]
Message-ID: <Y9k921b19TF6KDPE@nanopsycho> (raw)
In-Reply-To: <20230131145822.36208-6-alejandro.lucero-palau@amd.com>
Tue, Jan 31, 2023 at 03:58:19PM CET, alejandro.lucero-palau@amd.com wrote:
>From: Alejandro Lucero <alejandro.lucero-palau@amd.com>
>
>Using the data when enumerating mports, create devlink ports just before
>netdevs are registered and remove those devlink ports after netdev has
>been unregistered.
>
>Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
>---
> drivers/net/ethernet/sfc/ef100_netdev.c | 9 +++
> drivers/net/ethernet/sfc/ef100_rep.c | 22 ++++++
> drivers/net/ethernet/sfc/ef100_rep.h | 7 ++
> drivers/net/ethernet/sfc/efx_devlink.c | 97 +++++++++++++++++++++++++
> drivers/net/ethernet/sfc/efx_devlink.h | 6 ++
> drivers/net/ethernet/sfc/mae.h | 2 +
> drivers/net/ethernet/sfc/net_driver.h | 2 +
> 7 files changed, 145 insertions(+)
>
>diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
>index b10a226f4a07..36774b55d413 100644
>--- a/drivers/net/ethernet/sfc/ef100_netdev.c
>+++ b/drivers/net/ethernet/sfc/ef100_netdev.c
>@@ -335,7 +335,9 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
>
> /* devlink lock */
> efx_fini_devlink_start(efx);
>+
Does not seem related to the patch.
> ef100_unregister_netdev(efx);
>+ ef100_pf_unset_devlink_port(efx);
>
> #ifdef CONFIG_SFC_SRIOV
> efx_fini_tc(efx);
>@@ -423,6 +425,8 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
> rc = ef100_probe_netdev_pf(efx);
> if (rc)
> goto fail;
>+
>+ ef100_pf_set_devlink_port(efx);
> }
>
> efx->netdev_notifier.notifier_call = ef100_netdev_event;
>@@ -433,7 +437,12 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
> goto fail;
> }
>
>+ /* devlink unlock */
>+ efx_probe_devlink_done(efx);
>+ return rc;
> fail:
>+ /* remove devlink port if does exist */
>+ ef100_pf_unset_devlink_port(efx);
> /* devlink unlock */
> efx_probe_devlink_done(efx);
> return rc;
>diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
>index 9cd1a3ac67e0..6b5bc5d6955d 100644
>--- a/drivers/net/ethernet/sfc/ef100_rep.c
>+++ b/drivers/net/ethernet/sfc/ef100_rep.c
>@@ -16,6 +16,7 @@
> #include "mae.h"
> #include "rx_common.h"
> #include "tc_bindings.h"
>+#include "efx_devlink.h"
>
> #define EFX_EF100_REP_DRIVER "efx_ef100_rep"
>
>@@ -297,6 +298,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
> i, rc);
> goto fail1;
> }
>+ ef100_rep_set_devlink_port(efv);
> rc = register_netdev(efv->net_dev);
> if (rc) {
> pci_err(efx->pci_dev,
>@@ -308,6 +310,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
> efv->net_dev->name);
> return 0;
> fail2:
>+ ef100_rep_unset_devlink_port(efv);
> efx_ef100_deconfigure_rep(efv);
> fail1:
> efx_ef100_rep_destroy_netdev(efv);
>@@ -323,6 +326,7 @@ void efx_ef100_vfrep_destroy(struct efx_nic *efx, struct efx_rep *efv)
> return;
> netif_dbg(efx, drv, rep_dev, "Removing VF representor\n");
> unregister_netdev(rep_dev);
>+ ef100_rep_unset_devlink_port(efv);
> efx_ef100_deconfigure_rep(efv);
> efx_ef100_rep_destroy_netdev(efv);
> }
>@@ -339,6 +343,24 @@ void efx_ef100_fini_vfreps(struct efx_nic *efx)
> efx_ef100_vfrep_destroy(efx, efv);
> }
>
>+static bool ef100_mport_is_pcie_vnic(struct mae_mport_desc *mport_desc)
>+{
>+ return mport_desc->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
>+ mport_desc->vnic_client_type == MAE_MPORT_DESC_VNIC_CLIENT_TYPE_FUNCTION;
>+}
>+
>+bool ef100_mport_on_local_intf(struct efx_nic *efx,
>+ struct mae_mport_desc *mport_desc)
>+{
>+ struct ef100_nic_data *nic_data = efx->nic_data;
>+ bool pcie_func;
>+
>+ pcie_func = ef100_mport_is_pcie_vnic(mport_desc);
>+
>+ return nic_data->have_local_intf && pcie_func &&
>+ mport_desc->interface_idx == nic_data->local_mae_intf;
>+}
>+
> void efx_ef100_init_reps(struct efx_nic *efx)
> {
> struct ef100_nic_data *nic_data = efx->nic_data;
>diff --git a/drivers/net/ethernet/sfc/ef100_rep.h b/drivers/net/ethernet/sfc/ef100_rep.h
>index 328ac0cbb532..ae6add4b0855 100644
>--- a/drivers/net/ethernet/sfc/ef100_rep.h
>+++ b/drivers/net/ethernet/sfc/ef100_rep.h
>@@ -22,6 +22,8 @@ struct efx_rep_sw_stats {
> atomic64_t rx_dropped, tx_errors;
> };
>
>+struct devlink_port;
>+
> /**
> * struct efx_rep - Private data for an Efx representor
> *
>@@ -39,6 +41,7 @@ struct efx_rep_sw_stats {
> * @rx_lock: protects @rx_list
> * @napi: NAPI control structure
> * @stats: software traffic counters for netdev stats
>+ * @dl_port: devlink port associated to this netdev representor
> */
> struct efx_rep {
> struct efx_nic *parent;
>@@ -54,6 +57,7 @@ struct efx_rep {
> spinlock_t rx_lock;
> struct napi_struct napi;
> struct efx_rep_sw_stats stats;
>+ struct devlink_port *dl_port;
> };
>
> int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i);
>@@ -69,4 +73,7 @@ struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
> extern const struct net_device_ops efx_ef100_rep_netdev_ops;
> void efx_ef100_init_reps(struct efx_nic *efx);
> void efx_ef100_fini_reps(struct efx_nic *efx);
>+struct mae_mport_desc;
>+bool ef100_mport_on_local_intf(struct efx_nic *efx,
>+ struct mae_mport_desc *mport_desc);
> #endif /* EF100_REP_H */
>diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
>index ff5adfe3905e..b1637eb372ad 100644
>--- a/drivers/net/ethernet/sfc/efx_devlink.c
>+++ b/drivers/net/ethernet/sfc/efx_devlink.c
>@@ -16,11 +16,48 @@
> #include "mcdi.h"
> #include "mcdi_functions.h"
> #include "mcdi_pcol.h"
>+#include "mae.h"
>+#include "ef100_rep.h"
>
> struct efx_devlink {
> struct efx_nic *efx;
> };
>
>+static void efx_devlink_del_port(struct devlink_port *dl_port)
>+{
>+ if (!dl_port)
>+ return;
>+ devl_port_unregister(dl_port);
>+}
>+
>+static int efx_devlink_add_port(struct efx_nic *efx,
>+ struct mae_mport_desc *mport)
>+{
>+ bool external = false;
>+
>+ if (!ef100_mport_on_local_intf(efx, mport))
>+ external = true;
>+
>+ switch (mport->mport_type) {
>+ case MAE_MPORT_DESC_MPORT_TYPE_VNIC:
>+ if (mport->vf_idx != MAE_MPORT_DESC_VF_IDX_NULL)
>+ devlink_port_attrs_pci_vf_set(&mport->dl_port, 0, mport->pf_idx,
>+ mport->vf_idx,
>+ external);
>+ else
>+ devlink_port_attrs_pci_pf_set(&mport->dl_port, 0, mport->pf_idx,
>+ external);
>+ break;
>+ default:
>+ /* MAE_MPORT_DESC_MPORT_ALIAS and UNDEFINED */
>+ return 0;
>+ }
>+
>+ mport->dl_port.index = mport->mport_id;
>+
>+ return devl_port_register(efx->devlink, &mport->dl_port, mport->mport_id);
>+}
>+
> static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
> struct devlink_info_req *req,
> unsigned int partition_type,
>@@ -428,6 +465,66 @@ static const struct devlink_ops sfc_devlink_ops = {
> .info_get = efx_devlink_info_get,
> };
>
>+static struct devlink_port *ef100_set_devlink_port(struct efx_nic *efx, u32 idx)
>+{
>+ struct mae_mport_desc *mport;
>+ u32 id;
>+
>+ if (efx_mae_lookup_mport(efx, idx, &id)) {
>+ /* This should not happen. */
>+ if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>+ pci_warn(efx->pci_dev, "No mport ID found for PF.\n");
>+ else
>+ pci_warn(efx->pci_dev, "No mport ID found for VF %u.\n",
>+ idx);
>+ return NULL;
>+ }
>+
>+ mport = efx_mae_get_mport(efx, id);
>+ if (!mport) {
>+ /* This should not happen. */
Should not happen? WARN_ON?
>+ if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>+ pci_warn(efx->pci_dev, "No mport found for PF.\n");
>+ else
>+ pci_warn(efx->pci_dev, "No mport found for VF %u.\n",
>+ idx);
>+ return NULL;
>+ }
>+
>+ if (efx_devlink_add_port(efx, mport)) {
Store the return value into variable please.
>+ if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>+ pci_warn(efx->pci_dev,
>+ "devlink port creation for PF failed.\n");
>+ else
>+ pci_warn(efx->pci_dev,
>+ "devlink_port creationg for VF %u failed.\n",
>+ idx);
>+ return NULL;
>+ }
>+
>+ return &mport->dl_port;
>+}
>+
>+void ef100_rep_set_devlink_port(struct efx_rep *efv)
>+{
>+ efv->dl_port = ef100_set_devlink_port(efv->parent, efv->idx);
>+}
>+
>+void ef100_pf_set_devlink_port(struct efx_nic *efx)
>+{
>+ efx->dl_port = ef100_set_devlink_port(efx, MAE_MPORT_DESC_VF_IDX_NULL);
>+}
>+
>+void ef100_rep_unset_devlink_port(struct efx_rep *efv)
>+{
>+ efx_devlink_del_port(efv->dl_port);
>+}
>+
>+void ef100_pf_unset_devlink_port(struct efx_nic *efx)
>+{
>+ efx_devlink_del_port(efx->dl_port);
>+}
>+
> void efx_fini_devlink_start(struct efx_nic *efx)
> {
> if (efx->devlink)
>diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
>index 8bcd077d8d8d..d453a180c44c 100644
>--- a/drivers/net/ethernet/sfc/efx_devlink.h
>+++ b/drivers/net/ethernet/sfc/efx_devlink.h
>@@ -36,4 +36,10 @@ void efx_probe_devlink_done(struct efx_nic *efx);
> void efx_fini_devlink_start(struct efx_nic *efx);
> void efx_fini_devlink(struct efx_nic *efx);
>
>+struct efx_rep;
>+
>+void ef100_pf_set_devlink_port(struct efx_nic *efx);
>+void ef100_rep_set_devlink_port(struct efx_rep *efv);
>+void ef100_pf_unset_devlink_port(struct efx_nic *efx);
>+void ef100_rep_unset_devlink_port(struct efx_rep *efv);
> #endif /* _EFX_DEVLINK_H */
>diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
>index d9adeafc0654..e1b7967132ad 100644
>--- a/drivers/net/ethernet/sfc/mae.h
>+++ b/drivers/net/ethernet/sfc/mae.h
>@@ -13,6 +13,7 @@
> #define EF100_MAE_H
> /* MCDI interface for the ef100 Match-Action Engine */
>
>+#include <net/devlink.h>
> #include "net_driver.h"
> #include "tc.h"
> #include "mcdi_pcol.h" /* needed for various MC_CMD_MAE_*_NULL defines */
>@@ -44,6 +45,7 @@ struct mae_mport_desc {
> };
> struct rhash_head linkage;
> struct efx_rep *efv;
>+ struct devlink_port dl_port;
This is quite odd to store the devlink port struct in something called
"match action engine"... Why don't you put it to struct efx_nic
directly? Much more suitable.
> };
>
> int efx_mae_enumerate_mports(struct efx_nic *efx);
>diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
>index bc9efbfb3d6b..fcd51d3992fa 100644
>--- a/drivers/net/ethernet/sfc/net_driver.h
>+++ b/drivers/net/ethernet/sfc/net_driver.h
>@@ -998,6 +998,7 @@ struct efx_mae;
> * @netdev_notifier: Netdevice notifier.
> * @tc: state for TC offload (EF100).
> * @devlink: reference to devlink structure owned by this device
>+ * @dl_port: devlink port associated with the PF
> * @mem_bar: The BAR that is mapped into membase.
> * @reg_base: Offset from the start of the bar to the function control window.
> * @monitor_work: Hardware monitor workitem
>@@ -1185,6 +1186,7 @@ struct efx_nic {
> struct efx_tc_state *tc;
>
> struct devlink *devlink;
>+ struct devlink_port *dl_port;
> unsigned int mem_bar;
> u32 reg_base;
>
>--
>2.17.1
>
next prev parent reply other threads:[~2023-01-31 16:12 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 14:58 [PATCH v4 net-next 0/8] sfc: devlink support for ef100 alejandro.lucero-palau
2023-01-31 14:58 ` [PATCH v4 net-next 1/8] sfc: add " alejandro.lucero-palau
2023-01-31 16:00 ` Jiri Pirko
2023-02-01 8:49 ` Lucero Palau, Alejandro
2023-02-01 9:07 ` Jiri Pirko
2023-02-01 19:01 ` Jakub Kicinski
2023-02-02 6:41 ` Lucero Palau, Alejandro
2023-02-02 9:24 ` Martin Habets
2023-02-02 17:43 ` Jakub Kicinski
2023-01-31 14:58 ` [PATCH v4 net-next 2/8] sfc: add devlink info " alejandro.lucero-palau
2023-01-31 16:03 ` Jiri Pirko
2023-02-01 9:09 ` Lucero Palau, Alejandro
2023-02-01 12:03 ` Jiri Pirko
2023-01-31 14:58 ` [PATCH v4 net-next 3/8] sfc: enumerate mports in ef100 alejandro.lucero-palau
2023-02-01 11:12 ` kernel test robot
2023-01-31 14:58 ` [PATCH v4 net-next 4/8] sfc: add mport lookup based on driver's mport data alejandro.lucero-palau
2023-01-31 14:58 ` [PATCH v4 net-next 5/8] sfc: add devlink port support for ef100 alejandro.lucero-palau
2023-01-31 16:12 ` Jiri Pirko [this message]
2023-02-01 9:03 ` Lucero Palau, Alejandro
2023-02-01 9:08 ` Jiri Pirko
2023-02-01 14:57 ` kernel test robot
2023-01-31 14:58 ` [PATCH v4 net-next 6/8] sfc: obtain device mac address based on firmware handle " alejandro.lucero-palau
2023-01-31 14:58 ` [PATCH v4 net-next 7/8] sfc: add support for devlink port_function_hw_addr_get in ef100 alejandro.lucero-palau
2023-02-01 19:15 ` kernel test robot
2023-01-31 14:58 ` [PATCH v4 net-next 8/8] sfc: add support for devlink port_function_hw_addr_set " alejandro.lucero-palau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9k921b19TF6KDPE@nanopsycho \
--to=jiri@resnulli.us \
--cc=alejandro.lucero-palau@amd.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=ecree.xilinx@gmail.com \
--cc=edumazet@google.com \
--cc=habetsm.xilinx@gmail.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-net-drivers@amd.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).