RE: [EXTERNAL] Re: [net-next PATCH 3/9] octeontx2-pf: Create representor netdev

[Geethasowjanya Akula]

Hi Kalesh,



> -----Original Message-----
> From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@xxxxxxxxxxxx>
> Sent: Wednesday, April 17, 2024 9:58 AM
> To: Geethasowjanya Akula <gakula@xxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kuba@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx; pabeni@xxxxxxxxxx; edumazet@xxxxxxxxxx; Sunil
> Kovvuri Goutham <sgoutham@xxxxxxxxxxx>; Subbaraya Sundeep Bhatta
> <sbhatta@xxxxxxxxxxx>; Hariprasad Kelam <hkelam@xxxxxxxxxxx>
> Subject: [EXTERNAL] Re: [net-next PATCH 3/9] octeontx2-pf: Create
> representor netdev
> ----------------------------------------------------------------------
> On Tue, Apr 16, 2024 at 10:37 AM Geetha sowjanya <gakula@xxxxxxxxxxx>
> wrote:
> >
> > Adds initial devlink support to set/get the switchdev mode.
> > Representor netdevs are created for each rvu devices when
> > the switch mode is set to 'switchdev'. These netdevs are
> > be used to control and configure VFs.
> >
> > Signed-off-by: Geetha sowjanya <gakula@xxxxxxxxxxx>
> > ---
> >  .../marvell/octeontx2/nic/otx2_devlink.c      |  48 ++++++
> >  .../net/ethernet/marvell/octeontx2/nic/rep.c  | 159 ++++++++++++++++++
> >  .../net/ethernet/marvell/octeontx2/nic/rep.h  |   2 +
> >  3 files changed, 209 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
> > index 4e1130496573..f4f5f5d93c7e 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
> > @@ -76,7 +76,53 @@ static const struct devlink_param otx2_dl_params[] =
> {
> >                              otx2_dl_mcam_count_validate),
> >  };
> >
> > +#ifdef CONFIG_RVU_ESWITCH
> > +static int otx2_devlink_eswitch_mode_get(struct devlink *devlink, u16
> *mode)
> > +{
> > +       struct otx2_devlink *otx2_dl = devlink_priv(devlink);
> > +       struct otx2_nic *pfvf = otx2_dl->pfvf;
> > +
> > +       if (!is_rep_dev(pfvf->pdev))
> > +               return -EOPNOTSUPP;
> > +
> > +       *mode = pfvf->esw_mode;
> > +
> > +       return 0;
> > +}
> > +
> > +static int otx2_devlink_eswitch_mode_set(struct devlink *devlink, u16
> mode,
> > +                                        struct netlink_ext_ack *extack)
> > +{
> > +       struct otx2_devlink *otx2_dl = devlink_priv(devlink);
> > +       struct otx2_nic *pfvf = otx2_dl->pfvf;
> > +
> > +       if (!is_rep_dev(pfvf->pdev))
> > +               return -EOPNOTSUPP;
> > +
> > +       if (pfvf->esw_mode == mode)
> > +               return 0;
> > +
> > +       pfvf->esw_mode = mode;
> [Kalesh] Move this assignment after the switch block. Else, you will
> end up updating pfvf->esw_mode to an unsupported mode even though
> drover returns error.
Ack.
> > +       switch (mode) {
> > +       case DEVLINK_ESWITCH_MODE_LEGACY:
> > +               rvu_rep_destroy(pfvf);
> > +               break;
> > +       case DEVLINK_ESWITCH_MODE_SWITCHDEV:
> > +               rvu_rep_create(pfvf);
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> >  static const struct devlink_ops otx2_devlink_ops = {
> > +#ifdef CONFIG_RVU_ESWITCH
> > +       .eswitch_mode_get = otx2_devlink_eswitch_mode_get,
> > +       .eswitch_mode_set = otx2_devlink_eswitch_mode_set,
> > +#endif
> >  };
> >
> >  int otx2_register_dl(struct otx2_nic *pfvf)
> > @@ -112,6 +158,7 @@ int otx2_register_dl(struct otx2_nic *pfvf)
> >         devlink_free(dl);
> >         return err;
> >  }
> > +EXPORT_SYMBOL(otx2_register_dl);
> >
> >  void otx2_unregister_dl(struct otx2_nic *pfvf)
> >  {
> > @@ -123,3 +170,4 @@ void otx2_unregister_dl(struct otx2_nic *pfvf)
> >                                   ARRAY_SIZE(otx2_dl_params));
> >         devlink_free(dl);
> >  }
> > +EXPORT_SYMBOL(otx2_unregister_dl);
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > index b892a7fe3ddc..fd55ef40c934 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > @@ -28,6 +28,159 @@ MODULE_DESCRIPTION(DRV_STRING);
> >  MODULE_LICENSE("GPL");
> >  MODULE_DEVICE_TABLE(pci, rvu_rep_id_table);
> >
> > +static int rvu_rep_napi_init(struct otx2_nic *priv)
> > +{
> > +       struct otx2_cq_poll *cq_poll = NULL;
> > +       struct otx2_qset *qset = &priv->qset;
> > +       struct otx2_hw *hw = &priv->hw;
> > +       int err = 0, qidx, vec;
> > +       char *irq_name;
> > +
> > +       qset->napi = kcalloc(hw->cint_cnt, sizeof(*cq_poll), GFP_KERNEL);
> > +       if (!qset->napi)
> > +               return -ENOMEM;
> > +
> > +       /* Register NAPI handler */
> > +       for (qidx = 0; qidx < hw->cint_cnt; qidx++) {
> > +               cq_poll = &qset->napi[qidx];
> > +               cq_poll->cint_idx = qidx;
> > +               cq_poll->cq_ids[CQ_RX] =
> > +                       (qidx <  hw->rx_queues) ? qidx : CINT_INVALID_CQ;
> > +               cq_poll->cq_ids[CQ_TX] = (qidx < hw->tx_queues) ?
> > +                                         qidx + hw->rx_queues : CINT_INVALID_CQ;
> > +               cq_poll->cq_ids[CQ_XDP] = CINT_INVALID_CQ;
> > +               cq_poll->cq_ids[CQ_QOS] = CINT_INVALID_CQ;
> > +
> > +               cq_poll->dev = (void *)priv;
> > +               netif_napi_add(priv->reps[qidx]->netdev, &cq_poll->napi,
> > +                              otx2_napi_handler);
> > +               napi_enable(&cq_poll->napi);
> > +       }
> > +       /* Register CQ IRQ handlers */
> > +       vec = hw->nix_msixoff + NIX_LF_CINT_VEC_START;
> > +       for (qidx = 0; qidx < hw->cint_cnt; qidx++) {
> > +               irq_name = &hw->irq_name[vec * NAME_SIZE];
> > +
> > +               snprintf(irq_name, NAME_SIZE, "rep%d-rxtx-%d", qidx, qidx);
> > +
> > +               err = request_irq(pci_irq_vector(priv->pdev, vec),
> > +                                 otx2_cq_intr_handler, 0, irq_name,
> > +                                 &qset->napi[qidx]);
> > +               if (err) {
> > +                       dev_err(priv->dev,
> > +                               "RVU REP IRQ registration failed for CQ%d\n", qidx);
> > +                       goto err_free_cints;
> > +               }
> > +               vec++;
> > +
> > +               /* Enable CQ IRQ */
> > +               otx2_write64(priv, NIX_LF_CINTX_INT(qidx), BIT_ULL(0));
> > +               otx2_write64(priv, NIX_LF_CINTX_ENA_W1S(qidx), BIT_ULL(0));
> > +       }
> > +       priv->flags &= ~OTX2_FLAG_INTF_DOWN;
> > +       return 0;
> > +
> > +err_free_cints:
> > +       otx2_free_cints(priv, qidx);
> > +       otx2_disable_napi(priv);
> > +       return err;
> > +}
> > +
> > +static void rvu_rep_free_cq_rsrc(struct otx2_nic *priv)
> > +{
> > +       struct otx2_cq_poll *cq_poll = NULL;
> > +       struct otx2_qset *qset = &priv->qset;
> > +       int qidx, vec;
> > +
> > +       /* Cleanup CQ NAPI and IRQ */
> > +       vec = priv->hw.nix_msixoff + NIX_LF_CINT_VEC_START;
> > +       for (qidx = 0; qidx < priv->hw.cint_cnt; qidx++) {
> > +               /* Disable interrupt */
> > +               otx2_write64(priv, NIX_LF_CINTX_ENA_W1C(qidx), BIT_ULL(0));
> > +
> > +               synchronize_irq(pci_irq_vector(priv->pdev, vec));
> > +
> > +               cq_poll = &qset->napi[qidx];
> > +               napi_synchronize(&cq_poll->napi);
> > +               vec++;
> > +       }
> > +       otx2_free_cints(priv, priv->hw.cint_cnt);
> > +       otx2_disable_napi(priv);
> > +}
> > +
> > +static void rvu_rep_free_netdev(struct otx2_nic *priv)
> > +{
> > +       struct rep_dev *rep;
> > +       int rep_id;
> > +
> > +       for (rep_id = 0; rep_id < priv->rep_cnt; rep_id++) {
> > +               rep = priv->reps[rep_id];
> > +               if (rep && rep->netdev->netdev_ops) {
> > +                       unregister_netdev(rep->netdev);
> > +                       free_netdev(rep->netdev);
> > +               }
> > +       }
> > +       devm_kfree(priv->dev, priv->reps);
> > +}
> > +
> > +void rvu_rep_destroy(struct otx2_nic *priv)
> > +{
> > +       rvu_rep_free_cq_rsrc(priv);
> > +       rvu_rep_free_netdev(priv);
> > +}
> > +
> > +int rvu_rep_create(struct otx2_nic *priv)
> > +{
> > +       int rep_cnt = priv->rep_cnt;
> > +       struct net_device *ndev;
> > +       struct rep_dev *rep;
> > +       int rep_id, err;
> > +       u16 pcifunc;
> > +
> > +       priv->reps = devm_kcalloc(priv->dev, rep_cnt, sizeof(struct rep_dev),
> GFP_KERNEL);
> > +       if (!priv->reps)
> > +               return -ENOMEM;
> > +
> > +       for (rep_id = 0; rep_id < rep_cnt; rep_id++) {
> > +               ndev = alloc_etherdev(sizeof(*rep));
> > +               if (!ndev) {
> > +                       dev_err(priv->dev, "PFVF representor:%d creation failed\n",
> rep_id);
> > +                       err = -ENOMEM;
> > +                       goto exit;
> > +               }
> > +
> > +               rep = netdev_priv(ndev);
> > +               priv->reps[rep_id] = rep;
> > +               rep->mdev = priv;
> > +               rep->netdev = ndev;
> > +               rep->rep_id = rep_id;
> > +
> > +               ndev->min_mtu = OTX2_MIN_MTU;
> > +               ndev->max_mtu = priv->hw.max_mtu;
> > +               pcifunc = priv->rep_pf_map[rep_id];
> > +               rep->pcifunc = pcifunc;
> > +
> > +               snprintf(ndev->name, sizeof(ndev->name), "r%dp%dv%d", rep_id,
> > +                        rvu_get_pf(pcifunc), (pcifunc & RVU_PFVF_FUNC_MASK));
> > +
> > +               eth_hw_addr_random(ndev);
> > +               if (register_netdev(ndev)) {
> > +                       dev_err(priv->dev, "PFVF reprentator registration failed\n");
> > +                       free_netdev(ndev);
> > +                       ndev->netdev_ops = NULL;
> [Kalesh] update "rc" with the return status of register_netdev()
Ack.  Thanks for reviewing. Will submit the v2 with the changes. 
> > +                       goto exit;
> > +               }
> > +       }
> > +       err = rvu_rep_napi_init(priv);
> > +       if (err)
> > +               goto exit;
> > +
> > +       return 0;
> > +exit:
> > +       rvu_rep_free_netdev(priv);
> > +       return err;
> > +}
> > +
> >  static int rvu_rep_rsrc_free(struct otx2_nic *priv)
> >  {
> >         struct otx2_qset *qset = &priv->qset;
> > @@ -162,6 +315,10 @@ static int rvu_rep_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> >         if (err)
> >                 goto err_detach_rsrc;
> >
> > +       err = otx2_register_dl(priv);
> > +       if (err)
> > +               goto err_detach_rsrc;
> > +
> >         return 0;
> >
> >  err_detach_rsrc:
> > @@ -183,6 +340,8 @@ static void rvu_rep_remove(struct pci_dev *pdev)
> >  {
> >         struct otx2_nic *priv = pci_get_drvdata(pdev);
> >
> > +       otx2_unregister_dl(priv);
> > +       rvu_rep_destroy(priv);
> >         rvu_rep_rsrc_free(priv);
> >         otx2_detach_resources(&priv->mbox);
> >         if (priv->hw.lmt_info)
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
> b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
> > index 30cce17eb48b..be6c939e5cba 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
> > @@ -29,4 +29,6 @@ static inline bool is_rep_dev(struct pci_dev *pdev)
> >         return pdev->device == PCI_DEVID_RVU_REP;
> >  }
> >
> > +int rvu_rep_create(struct otx2_nic *priv);
> > +void rvu_rep_destroy(struct otx2_nic *priv);
> >  #endif /* REP_H */
> > --
> > 2.25.1
> >
> >
> 
> 
> --
> Regards,
> Kalesh A P