| 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
| ||
|
Message-ID: <1fb1d09b719548f0a2bad261f2bf9b4a@realtek.com>
Date: Thu, 12 Jun 2025 03:28:04 +0000
From: Justin Lai <justinlai0215@...ltek.com>
To: Joe Damato <joe@...a.to>
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>,
"horms@...nel.org" <horms@...nel.org>,
"jdamato@...tly.com" <jdamato@...tly.com>,
Ping-Ke Shih <pkshih@...ltek.com>, Larry Chiu <larry.chiu@...ltek.com>
Subject: RE: [PATCH net-next 2/2] rtase: Link queues to NAPI instances
> On Tue, Jun 10, 2025 at 06:33:34PM +0800, Justin Lai wrote:
> > Link queues to NAPI instances with netif_queue_set_napi. This
> > information can be queried with the netdev-genl API.
> >
> > Signed-off-by: Justin Lai <justinlai0215@...ltek.com>
> > ---
> > drivers/net/ethernet/realtek/rtase/rtase.h | 4 +++
> > .../net/ethernet/realtek/rtase/rtase_main.c | 33 +++++++++++++++++--
> > 2 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h
> > b/drivers/net/ethernet/realtek/rtase/rtase.h
> > index 498cfe4d0cac..be98f4de46c4 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase.h
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase.h
> > @@ -39,6 +39,9 @@
> > #define RTASE_FUNC_RXQ_NUM 1
> > #define RTASE_INTERRUPT_NUM 1
> >
> > +#define RTASE_TX_RING 0
> > +#define RTASE_RX_RING 1
> > +
> > #define RTASE_MITI_TIME_COUNT_MASK GENMASK(3, 0)
> > #define RTASE_MITI_TIME_UNIT_MASK GENMASK(7, 4)
> > #define RTASE_MITI_DEFAULT_TIME 128
> > @@ -288,6 +291,7 @@ struct rtase_ring {
> > u32 cur_idx;
> > u32 dirty_idx;
> > u16 index;
> > + u8 ring_type;
>
> Two questions:
>
> 1. why not use enum netdev_queue_type instead of making driver specific
> values that are the opposite of the existing enum values ?
>
> If you used the existing enum, you could omit the if below (see below), as
> well?
>
> 2. is "ring" in the name redundant? maybe just "type"? asking because below
> the code becomes "ring->ring_type" and maybe "ring->type" is better?
>
> > struct sk_buff *skbuff[RTASE_NUM_DESC];
> > void *data_buf[RTASE_NUM_DESC];
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index a88af868da8c..ef3ada91d555 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > @@ -326,6 +326,7 @@ static void rtase_tx_desc_init(struct rtase_private
> *tp, u16 idx)
> > ring->cur_idx = 0;
> > ring->dirty_idx = 0;
> > ring->index = idx;
> > + ring->ring_type = RTASE_TX_RING;
> > ring->alloc_fail = 0;
> >
> > for (i = 0; i < RTASE_NUM_DESC; i++) { @@ -345,6 +346,10 @@
> > static void rtase_tx_desc_init(struct rtase_private *tp, u16 idx)
> > ring->ivec = &tp->int_vector[0];
> > list_add_tail(&ring->ring_entry,
> &tp->int_vector[0].ring_list);
> > }
> > +
> > + netif_queue_set_napi(tp->dev, ring->index,
> > + NETDEV_QUEUE_TYPE_TX,
> > + &ring->ivec->napi);
> > }
> >
> > static void rtase_map_to_asic(union rtase_rx_desc *desc, dma_addr_t
> > mapping, @@ -590,6 +595,7 @@ static void rtase_rx_desc_init(struct
> rtase_private *tp, u16 idx)
> > ring->cur_idx = 0;
> > ring->dirty_idx = 0;
> > ring->index = idx;
> > + ring->ring_type = RTASE_RX_RING;
> > ring->alloc_fail = 0;
> >
> > for (i = 0; i < RTASE_NUM_DESC; i++) @@ -597,6 +603,9 @@ static
> > void rtase_rx_desc_init(struct rtase_private *tp, u16 idx)
> >
> > ring->ring_handler = rx_handler;
> > ring->ivec = &tp->int_vector[idx];
> > + netif_queue_set_napi(tp->dev, ring->index,
> > + NETDEV_QUEUE_TYPE_RX,
> > + &ring->ivec->napi);
> > list_add_tail(&ring->ring_entry,
> > &tp->int_vector[idx].ring_list); }
> >
> > @@ -1161,8 +1170,18 @@ static void rtase_down(struct net_device *dev)
> > ivec = &tp->int_vector[i];
> > napi_disable(&ivec->napi);
> > list_for_each_entry_safe(ring, tmp, &ivec->ring_list,
> > - ring_entry)
> > + ring_entry) {
> > + if (ring->ring_type == RTASE_TX_RING)
> > + netif_queue_set_napi(tp->dev,
> ring->index,
> > +
> NETDEV_QUEUE_TYPE_TX,
> > + NULL);
> > + else
> > + netif_queue_set_napi(tp->dev,
> ring->index,
> > +
> NETDEV_QUEUE_TYPE_RX,
> > + NULL);
> > +
>
> Using the existing enum would simplify this block?
>
> netif_queue_set_napi(tp->dev, ring->index, ring->type, NULL);
>
> > list_del(&ring->ring_entry);
> > + }
> > }
> >
> > netif_tx_disable(dev);
> > @@ -1518,8 +1537,18 @@ static void rtase_sw_reset(struct net_device
> *dev)
> > for (i = 0; i < tp->int_nums; i++) {
> > ivec = &tp->int_vector[i];
> > list_for_each_entry_safe(ring, tmp, &ivec->ring_list,
> > - ring_entry)
> > + ring_entry) {
> > + if (ring->ring_type == RTASE_TX_RING)
> > + netif_queue_set_napi(tp->dev,
> ring->index,
> > +
> NETDEV_QUEUE_TYPE_TX,
> > + NULL);
> > + else
> > + netif_queue_set_napi(tp->dev,
> ring->index,
> > +
> NETDEV_QUEUE_TYPE_RX,
> > + NULL);
> > +
>
> Same as above?
Hi Joe,
Thank you for your reply. I will use the enum netdev_queue_type to avoid
needing if statements to determine the ring type later. I will also rename
ring_type to type.
Thanks,
Justin
Powered by blists - more mailing lists