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

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: <ab27fd1a1e9d40759e090346eafb5881@realtek.com>
Date: Mon, 24 Mar 2025 12:06:09 +0000
From: Justin Lai <justinlai0215@...ltek.com>
To: Justin Lai <justinlai0215@...ltek.com>, Simon Horman <horms@...nel.org>
CC: "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>,
        Ping-Ke Shih <pkshih@...ltek.com>,
        Larry Chiu
	<larry.chiu@...ltek.com>
Subject: RE: [PATCH net-next] rtase: Add ndo_setup_tc support for CBS offload in traffic control setup

> 
> > 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@...ltek.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?

> >
> > > +
> > > +     tp->tx_qos[queue].hicredit = qopt->hicredit;
> > > +     tp->tx_qos[queue].locredit = qopt->locredit;
> > > +     tp->tx_qos[queue].idleslope = qopt->idleslope;
> > > +     tp->tx_qos[queue].sendslope = qopt->sendslope;
> >
> > Does qopt->enable need to be honoured in order to allow the offload to
> > be both enabled and disabled?
> >
> Thank you for your suggestion. I will add a check for qopt->enable and handle
> it appropriately.
> 
> > > +
> > > +     rtase_set_hw_cbs(tp, queue);
> > > +}
> > > +
> > > +static int rtase_setup_tc(struct net_device *dev, enum tc_setup_type
> type,
> > > +                       void *type_data) {
> > > +     struct rtase_private *tp = netdev_priv(dev);
> > > +
> > > +     switch (type) {
> > > +     case TC_SETUP_QDISC_CBS:
> > > +             rtase_setup_tc_cbs(tp, type_data);
> > > +             break;
> > > +     default:
> > > +             return -EOPNOTSUPP;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static netdev_features_t rtase_fix_features(struct net_device *dev,
> > >                                           netdev_features_t
> > features)
> > > {
> >
> > ...