Re: [PATCH net-next] rtase: Add ndo_setup_tc support for CBS offload in traffic control setup

[Simon Horman <horms@kernel.org>]

On Mon, Mar 24, 2025 at 12:06:09PM +0000, Justin Lai wrote:
> > 
> > > On Fri, Mar 14, 2025 at 05:40:21PM +0800, Justin Lai wrote:
> > > > Add support for ndo_setup_tc to enable CBS offload functionality as
> > > > part of traffic control configuration for network devices.
> > > >
> > > > Signed-off-by: Justin Lai <justinlai0215@realtek.com>
> > >
> > > ...
> > >
> > > > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > > index 2aacc1996796..2a61cd192026 100644
> > > > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > > @@ -1661,6 +1661,54 @@ static void rtase_get_stats64(struct
> > > > net_device
> > > *dev,
> > > >       stats->rx_length_errors = tp->stats.rx_length_errors;  }
> > > >
> > > > +static void rtase_set_hw_cbs(const struct rtase_private *tp, u32
> > > > +queue) {
> > > > +     u32 idle = tp->tx_qos[queue].idleslope * RTASE_1T_CLOCK;
> > > > +     u32 val, i;
> > > > +
> > > > +     val = u32_encode_bits(idle / RTASE_1T_POWER,
> > > RTASE_IDLESLOPE_INT_MASK);
> > > > +     idle %= RTASE_1T_POWER;
> > > > +
> > > > +     for (i = 1; i <= RTASE_IDLESLOPE_INT_SHIFT; i++) {
> > > > +             idle *= 2;
> > > > +             if ((idle / RTASE_1T_POWER) == 1)
> > > > +                     val |= BIT(RTASE_IDLESLOPE_INT_SHIFT - i);
> > > > +
> > > > +             idle %= RTASE_1T_POWER;
> > > > +     }
> > > > +
> > > > +     rtase_w32(tp, RTASE_TXQCRDT_0 + queue * 4, val); }
> > > > +
> > > > +static void rtase_setup_tc_cbs(struct rtase_private *tp,
> > > > +                            const struct tc_cbs_qopt_offload *qopt)
> > {
> > > > +     u32 queue = qopt->queue;
> > >
> > > Hi Justin,
> > >
> > > Does queue need to be checked somewhere to make sure it is in range?
> > 
> > Hi Simon,
> > 
> > Thank you for your response. I will add a check to ensure that the queue is
> > within the specified range.
> 
> Hi Simon,
> 
> Given that our hardware supports CBS offload on each queue, could it
> be possible that checking the range of qopt->queue might not be
> necessary?

Hi Justin,

If qopt->queue can only be a valid queue, and all queues support
CBS, then I guess it would not be necessary.