Re: [net-next Patch v5 5/6] octeontx2-pf: Add support for HTB offload

[Hariprasad Kelam]

Thanks for the review, will address issues in next version.

Thanks,
Hariprasad k

> On Sun, Mar 26, 2023 at 11:42:44PM +0530, Hariprasad Kelam wrote:
> > From: Naveen Mamindlapalli <naveenm@xxxxxxxxxxx>
> >
> > This patch registers callbacks to support HTB offload.
> >
> > Below are features supported,
> >
> > - supports traffic shaping on the given class by honoring rate and
> > ceil configuration.
> >
> > - supports traffic scheduling,  which prioritizes different types of
> > traffic based on strict priority values.
> >
> > - supports the creation of leaf to inner classes such that parent node
> > rate limits apply to all child nodes.
> >
> > Signed-off-by: Naveen Mamindlapalli <naveenm@xxxxxxxxxxx>
> > Signed-off-by: Hariprasad Kelam <hkelam@xxxxxxxxxxx>
> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@xxxxxxxxxxx>
> 
> Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
> 
> > ---
> >  .../ethernet/marvell/octeontx2/af/common.h    |    2 +-
> >  .../ethernet/marvell/octeontx2/nic/Makefile   |    2 +-
> >  .../marvell/octeontx2/nic/otx2_common.c       |   35 +-
> >  .../marvell/octeontx2/nic/otx2_common.h       |    8 +-
> >  .../marvell/octeontx2/nic/otx2_ethtool.c      |   31 +-
> >  .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |   56 +-
> >  .../ethernet/marvell/octeontx2/nic/otx2_reg.h |   13 +
> >  .../ethernet/marvell/octeontx2/nic/otx2_tc.c  |    7 +-
> >  .../net/ethernet/marvell/octeontx2/nic/qos.c  | 1460
> +++++++++++++++++
> >  .../net/ethernet/marvell/octeontx2/nic/qos.h  |   58 +-
> >  .../ethernet/marvell/octeontx2/nic/qos_sq.c   |   20 +-
> >  11 files changed, 1657 insertions(+), 35 deletions(-)
> 
> nit: this is a rather long patch.
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> 
> ...
> 
> > @@ -159,7 +165,7 @@ static void otx2_get_qset_stats(struct otx2_nic
> *pfvf,
> >  				[otx2_queue_stats[stat].index];
> >  	}
> >
> > -	for (qidx = 0; qidx < pfvf->hw.tx_queues; qidx++) {
> > +	for (qidx = 0; qidx <  otx2_get_total_tx_queues(pfvf); qidx++) {
> 
> nit: extra whitespace after '<'
> 
> >  		if (!otx2_update_sq_stats(pfvf, qidx)) {
> >  			for (stat = 0; stat < otx2_n_queue_stats; stat++)
> >  				*((*data)++) = 0;
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> > b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> 
> ...
> 
> > +static int otx2_qos_update_tx_netdev_queues(struct otx2_nic *pfvf) {
> > +	int tx_queues, qos_txqs, err;
> > +	struct otx2_hw *hw = &pfvf->hw;
> 
> nit: reverse xmas tree - longest line to shortest -
>      for local variable declarations.
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.h
> > b/drivers/net/ethernet/marvell/octeontx2/nic/qos.h
> 
> ...
> 
> > +struct otx2_qos_node {
> > +	struct list_head list; /* list managment */
> 
> nit: s/managment/management/
> 
> > +	struct list_head child_list;
> > +	struct list_head child_schq_list;
> > +	struct hlist_node hlist;
> > +	DECLARE_BITMAP(prio_bmap, OTX2_QOS_MAX_PRIO + 1);
> > +	struct otx2_qos_node *parent;	/* parent qos node */
> > +	u64 rate; /* htb params */
> > +	u64 ceil;
> > +	u32 classid;
> > +	u32 prio;
> > +	u16 schq; /* hw txschq */
> > +	u16 qid;
> > +	u16 prio_anchor;
> > +	u8 level;
> > +};
> > +
> >
> >  #endif
> 
> ...