linux-kernel - Re: [net-next PATCH 1/3] octeontx2-af: Add devlink suppoort to af driver

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 linux-cve-announce 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]

Message-ID: <CA+FuTSdXOmzSX3Y__hEa=gk4nqFQDur7m9VsDJJ_cYPp2Asi_g@mail.gmail.com>
Date:   Mon, 2 Nov 2020 08:31:03 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     George Cherian <george.cherian@...vell.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>, sgoutham@...vell.com,
        lcherian@...vell.com, gakula@...vell.com, masahiroy@...nel.org
Subject: Re: [net-next PATCH 1/3] octeontx2-af: Add devlink suppoort to af driver

On Mon, Nov 2, 2020 at 12:07 AM George Cherian
<george.cherian@...vell.com> wrote:
>
> Add devlink support to AF driver. Basic devlink support is added.
> Currently info_get is the only supported devlink ops.
>
> devlink ouptput looks like this
>  # devlink dev
>  pci/0002:01:00.0
>  # devlink dev info
>  pci/0002:01:00.0:
>   driver octeontx2-af
>   versions:
>       fixed:
>         mbox version: 9
>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@...vell.com>
> Signed-off-by: Jerin Jacob <jerinj@...vell.com>
> Signed-off-by: George Cherian <george.cherian@...vell.com>

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> index 5ac9bb12415f..c112b299635d 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> @@ -12,7 +12,10 @@
>  #define RVU_H
>
>  #include <linux/pci.h>
> +#include <net/devlink.h>
> +
>  #include "rvu_struct.h"
> +#include "rvu_devlink.h"
>  #include "common.h"
>  #include "mbox.h"
>
> @@ -372,10 +375,10 @@ struct rvu {
>         struct npc_kpu_profile_adapter kpu;
>
>         struct ptp              *ptp;
> -

accidentally removed this line?

>  #ifdef CONFIG_DEBUG_FS
>         struct rvu_debugfs      rvu_dbg;
>  #endif
> +       struct rvu_devlink      *rvu_dl;
>  };


> +int rvu_register_dl(struct rvu *rvu)
> +{
> +       struct rvu_devlink *rvu_dl;
> +       struct devlink *dl;
> +       int err;
> +
> +       rvu_dl = kzalloc(sizeof(*rvu_dl), GFP_KERNEL);
> +       if (!rvu_dl)
> +               return -ENOMEM;
> +
> +       dl = devlink_alloc(&rvu_devlink_ops, sizeof(struct rvu_devlink));
> +       if (!dl) {
> +               dev_warn(rvu->dev, "devlink_alloc failed\n");
> +               return -ENOMEM;

rvu_dl not freed on error.

This happens a couple of times in these patches

Is the intermediate struct needed, or could you embed the fields
directly into rvu and use container_of to get from devlink to struct
rvu? Even if needed, perhaps easier to embed the struct into rvu
rather than a pointer.

> +       }
> +
> +       err = devlink_register(dl, rvu->dev);
> +       if (err) {
> +               dev_err(rvu->dev, "devlink register failed with error %d\n", err);
> +               devlink_free(dl);
> +               return err;
> +       }
> +
> +       rvu_dl->dl = dl;
> +       rvu_dl->rvu = rvu;
> +       rvu->rvu_dl = rvu_dl;
> +       return 0;
> +}
> +
> +void rvu_unregister_dl(struct rvu *rvu)
> +{
> +       struct rvu_devlink *rvu_dl = rvu->rvu_dl;
> +       struct devlink *dl = rvu_dl->dl;
> +
> +       if (!dl)
> +               return;
> +
> +       devlink_unregister(dl);
> +       devlink_free(dl);

here too