[PATCH net-next 2/2] rtase: Link queues to NAPI instances

Justin Lai posted 2 patches 4 months ago
There is a newer version of this series
[PATCH net-next 2/2] rtase: Link queues to NAPI instances
Posted by Justin Lai 4 months ago
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@realtek.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;
 
 	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);
+
 			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);
+
 			list_del(&ring->ring_entry);
+		}
 	}
 
 	ret = rtase_init_ring(dev);
-- 
2.34.1
Re: [PATCH net-next 2/2] rtase: Link queues to NAPI instances
Posted by Joe Damato 4 months ago
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@realtek.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?
RE: [PATCH net-next 2/2] rtase: Link queues to NAPI instances
Posted by Justin Lai 4 months ago
> 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@realtek.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