lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 27 Jan 2020 10:21:06 +0530
From:   Vasundhara Volam <vasundhara-v.volam@...adcom.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Michael Chan <michael.chan@...adcom.com>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 09/16] bnxt_en: Refactor bnxt_dl_register()

On Sun, Jan 26, 2020 at 5:02 PM Jiri Pirko <jiri@...nulli.us> wrote:
>
> Sun, Jan 26, 2020 at 10:03:03AM CET, michael.chan@...adcom.com wrote:
> >From: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
> >
> >Define bnxt_dl_params_register() and bnxt_dl_params_unregister()
> >functions and move params register/unregister code to these newly
> >defined functions. This patch is in preparation to register
> >devlink irrespective of firmware spec. version in the next patch.
> >
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
> >Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> >---
> > drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 60 ++++++++++++++---------
> > 1 file changed, 36 insertions(+), 24 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> >index 0c3d224..9253eed 100644
> >--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> >+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> >@@ -485,6 +485,38 @@ static const struct devlink_param bnxt_dl_params[] = {
> > static const struct devlink_param bnxt_dl_port_params[] = {
> > };
> >
> >+static int bnxt_dl_params_register(struct bnxt *bp)
> >+{
> >+      int rc;
> >+
> >+      rc = devlink_params_register(bp->dl, bnxt_dl_params,
> >+                                   ARRAY_SIZE(bnxt_dl_params));
> >+      if (rc) {
> >+              netdev_warn(bp->dev, "devlink_params_register failed. rc=%d",
> >+                          rc);
> >+              return rc;
> >+      }
> >+      rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params,
> >+                                        ARRAY_SIZE(bnxt_dl_port_params));
>
> Wait, this assumes that you have 1:1 devlink:devlink_port setup. Is that
> correct? Don't you have other devlink_ports for eswitch representors
> that have params?
Yes Jiri, this assumes 1:1 setup. Our driver does not register params for VFs.
It will register params only for PFs.

This patch is refactoring of code and moving params_registers to a new function,
which will not be called for VFs.

There is a check for PF in bnxt_dl_register() and return before calling
bnxt_dl_params_register(), if check fails.
>
>
> >+      if (rc) {
> >+              netdev_err(bp->dev, "devlink_port_params_register failed");
> >+              devlink_params_unregister(bp->dl, bnxt_dl_params,
> >+                                        ARRAY_SIZE(bnxt_dl_params));
> >+              return rc;
> >+      }
> >+      devlink_params_publish(bp->dl);
> >+
> >+      return 0;
> >+}
> >+
> >+static void bnxt_dl_params_unregister(struct bnxt *bp)
> >+{
> >+      devlink_params_unregister(bp->dl, bnxt_dl_params,
> >+                                ARRAY_SIZE(bnxt_dl_params));
> >+      devlink_port_params_unregister(&bp->dl_port, bnxt_dl_port_params,
> >+                                     ARRAY_SIZE(bnxt_dl_port_params));
> >+}
> >+
> > int bnxt_dl_register(struct bnxt *bp)
> > {
> >       struct devlink *dl;
> >@@ -520,40 +552,24 @@ int bnxt_dl_register(struct bnxt *bp)
> >       if (!BNXT_PF(bp))
> >               return 0;
> >
> >-      rc = devlink_params_register(dl, bnxt_dl_params,
> >-                                   ARRAY_SIZE(bnxt_dl_params));
> >-      if (rc) {
> >-              netdev_warn(bp->dev, "devlink_params_register failed. rc=%d",
> >-                          rc);
> >-              goto err_dl_unreg;
> >-      }
> >-
> >       devlink_port_attrs_set(&bp->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
> >                              bp->pf.port_id, false, 0,
> >                              bp->switch_id, sizeof(bp->switch_id));
> >       rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
> >       if (rc) {
> >               netdev_err(bp->dev, "devlink_port_register failed");
> >-              goto err_dl_param_unreg;
> >+              goto err_dl_unreg;
> >       }
> >       devlink_port_type_eth_set(&bp->dl_port, bp->dev);
> >
> >-      rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params,
> >-                                        ARRAY_SIZE(bnxt_dl_port_params));
> >-      if (rc) {
> >-              netdev_err(bp->dev, "devlink_port_params_register failed");
> >+      rc = bnxt_dl_params_register(bp);
> >+      if (rc)
> >               goto err_dl_port_unreg;
> >-      }
> >-
> >-      devlink_params_publish(dl);
> >
> >       return 0;
> >
> > err_dl_port_unreg:
> >       devlink_port_unregister(&bp->dl_port);
> >-err_dl_param_unreg:
> >-      devlink_params_unregister(dl, bnxt_dl_params,
> >-                                ARRAY_SIZE(bnxt_dl_params));
> > err_dl_unreg:
> >       devlink_unregister(dl);
> > err_dl_free:
> >@@ -570,12 +586,8 @@ void bnxt_dl_unregister(struct bnxt *bp)
> >               return;
> >
> >       if (BNXT_PF(bp)) {
> >-              devlink_port_params_unregister(&bp->dl_port,
> >-                                             bnxt_dl_port_params,
> >-                                             ARRAY_SIZE(bnxt_dl_port_params));
> >+              bnxt_dl_params_unregister(bp);
> >               devlink_port_unregister(&bp->dl_port);
> >-              devlink_params_unregister(dl, bnxt_dl_params,
> >-                                        ARRAY_SIZE(bnxt_dl_params));
> >       }
> >       devlink_unregister(dl);
> >       devlink_free(dl);
> >--
> >2.5.1
> >

Powered by blists - more mailing lists