diff mbox series

[net-next,03/10] gve: Add adminq funcs to add/remove a single Rx queue

Message ID 20240430231420.699177-4-shailend@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series gve: Implement queue api | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 926 this patch: 926
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 940 this patch: 940
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 937 this patch: 937
netdev/checkpatch warning WARNING: line length of 94 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Shailend Chand April 30, 2024, 11:14 p.m. UTC 
This allows for implementing future ndo hooks that act on a single
queue.

Tested-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Signed-off-by: Shailend Chand <shailend@google.com>
---
 drivers/net/ethernet/google/gve/gve_adminq.c | 79 ++++++++++++++------
 drivers/net/ethernet/google/gve/gve_adminq.h |  2 +
 2 files changed, 58 insertions(+), 23 deletions(-)

Comments

Ratheesh Kannoth May 1, 2024, 4:19 a.m. UTC | #1 
On 2024-05-01 at 04:44:12, Shailend Chand (shailend@google.com) wrote:
> +int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> +{
> +	union gve_adminq_command cmd;
> +	int err;
> +
> +	gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> +	err = gve_adminq_execute_cmd(priv, &cmd);
> +	if (err)
why can't you return err directly ? no need of if statement.
> +		return err;

> +
> +	return 0;
>
Willem de Bruijn May 1, 2024, 1:50 p.m. UTC | #2 
Shailend Chand wrote:
> This allows for implementing future ndo hooks that act on a single
> queue.
> 
> Tested-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Signed-off-by: Shailend Chand <shailend@google.com>

> +static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
> +{
> +	union gve_adminq_command cmd;
> +
> +	gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
>  	return gve_adminq_issue_cmd(priv, &cmd);
>  }
>  
> +int gve_adminq_create_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> +{
> +	union gve_adminq_command cmd;
> +
> +	gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
> +	return gve_adminq_execute_cmd(priv, &cmd);
> +}
> +
>  int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues)
>  {
>  	int err;
> @@ -727,17 +742,22 @@ int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_qu
>  	return gve_adminq_kick_and_wait(priv);
>  }
>  
> +static void gve_adminq_make_destroy_rx_queue_cmd(union gve_adminq_command *cmd,
> +						 u32 queue_index)
> +{
> +	memset(cmd, 0, sizeof(*cmd));
> +	cmd->opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> +	cmd->destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> +		.queue_id = cpu_to_be32(queue_index),
> +	};
> +}
> +
>  static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
>  {
>  	union gve_adminq_command cmd;
>  	int err;
>  
> -	memset(&cmd, 0, sizeof(cmd));
> -	cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> -	cmd.destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> -		.queue_id = cpu_to_be32(queue_index),
> -	};
> -
> +	gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
>  	err = gve_adminq_issue_cmd(priv, &cmd);
>  	if (err)
>  		return err;
> @@ -745,6 +765,19 @@ static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
>  	return 0;
>  }
>  
> +int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> +{
> +	union gve_adminq_command cmd;
> +	int err;
> +
> +	gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> +	err = gve_adminq_execute_cmd(priv, &cmd);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}

This is identical to gve_adminq_destroy_rx_queue, bar for removing the
file scope?

Same for gve_adminq_create_rx_queue.
Shailend Chand May 1, 2024, 11:27 p.m. UTC | #3 
On Wed, May 1, 2024 at 6:50 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Shailend Chand wrote:
> > This allows for implementing future ndo hooks that act on a single
> > queue.
> >
> > Tested-by: Mina Almasry <almasrymina@google.com>
> > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> > Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > Signed-off-by: Shailend Chand <shailend@google.com>
>
> > +static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
> > +{
> > +     union gve_adminq_command cmd;
> > +
> > +     gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
> >       return gve_adminq_issue_cmd(priv, &cmd);
> >  }
> >
> > +int gve_adminq_create_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> > +{
> > +     union gve_adminq_command cmd;
> > +
> > +     gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
> > +     return gve_adminq_execute_cmd(priv, &cmd);
> > +}
> > +
> >  int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues)
> >  {
> >       int err;
> > @@ -727,17 +742,22 @@ int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_qu
> >       return gve_adminq_kick_and_wait(priv);
> >  }
> >
> > +static void gve_adminq_make_destroy_rx_queue_cmd(union gve_adminq_command *cmd,
> > +                                              u32 queue_index)
> > +{
> > +     memset(cmd, 0, sizeof(*cmd));
> > +     cmd->opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> > +     cmd->destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> > +             .queue_id = cpu_to_be32(queue_index),
> > +     };
> > +}
> > +
> >  static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
> >  {
> >       union gve_adminq_command cmd;
> >       int err;
> >
> > -     memset(&cmd, 0, sizeof(cmd));
> > -     cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> > -     cmd.destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> > -             .queue_id = cpu_to_be32(queue_index),
> > -     };
> > -
> > +     gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> >       err = gve_adminq_issue_cmd(priv, &cmd);
> >       if (err)
> >               return err;
> > @@ -745,6 +765,19 @@ static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
> >       return 0;
> >  }
> >
> > +int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> > +{
> > +     union gve_adminq_command cmd;
> > +     int err;
> > +
> > +     gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> > +     err = gve_adminq_execute_cmd(priv, &cmd);
> > +     if (err)
> > +             return err;
> > +
> > +     return 0;
> > +}
>
> This is identical to gve_adminq_destroy_rx_queue, bar for removing the
> file scope?
>
> Same for gve_adminq_create_rx_queue.

One doesn't immediately ring the doorbell, added a comment in v2
clarifying this.
Willem de Bruijn May 2, 2024, 1:39 a.m. UTC | #4 
Shailend Chand wrote:
> On Wed, May 1, 2024 at 6:50 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Shailend Chand wrote:
> > > This allows for implementing future ndo hooks that act on a single
> > > queue.
> > >
> > > Tested-by: Mina Almasry <almasrymina@google.com>
> > > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> > > Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > > Signed-off-by: Shailend Chand <shailend@google.com>
> >
> > > +static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
> > > +{
> > > +     union gve_adminq_command cmd;
> > > +
> > > +     gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
> > >       return gve_adminq_issue_cmd(priv, &cmd);
> > >  }
> > >
> > > +int gve_adminq_create_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> > > +{
> > > +     union gve_adminq_command cmd;
> > > +
> > > +     gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
> > > +     return gve_adminq_execute_cmd(priv, &cmd);
> > > +}
> > > +
> > >  int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues)
> > >  {
> > >       int err;
> > > @@ -727,17 +742,22 @@ int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_qu
> > >       return gve_adminq_kick_and_wait(priv);
> > >  }
> > >
> > > +static void gve_adminq_make_destroy_rx_queue_cmd(union gve_adminq_command *cmd,
> > > +                                              u32 queue_index)
> > > +{
> > > +     memset(cmd, 0, sizeof(*cmd));
> > > +     cmd->opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> > > +     cmd->destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> > > +             .queue_id = cpu_to_be32(queue_index),
> > > +     };
> > > +}
> > > +
> > >  static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
> > >  {
> > >       union gve_adminq_command cmd;
> > >       int err;
> > >
> > > -     memset(&cmd, 0, sizeof(cmd));
> > > -     cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
> > > -     cmd.destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
> > > -             .queue_id = cpu_to_be32(queue_index),
> > > -     };
> > > -
> > > +     gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> > >       err = gve_adminq_issue_cmd(priv, &cmd);
> > >       if (err)
> > >               return err;
> > > @@ -745,6 +765,19 @@ static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
> > >       return 0;
> > >  }
> > >
> > > +int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index)
> > > +{
> > > +     union gve_adminq_command cmd;
> > > +     int err;
> > > +
> > > +     gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
> > > +     err = gve_adminq_execute_cmd(priv, &cmd);
> > > +     if (err)
> > > +             return err;
> > > +
> > > +     return 0;
> > > +}
> >
> > This is identical to gve_adminq_destroy_rx_queue, bar for removing the
> > file scope?
> >
> > Same for gve_adminq_create_rx_queue.
> 
> One doesn't immediately ring the doorbell, added a comment in v2
> clarifying this.

Thanks! I clearly totally missed the issue vs execute distinction.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index b2b619aa2310..1b066c92d812 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -630,14 +630,15 @@  int gve_adminq_create_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_que
 	return gve_adminq_kick_and_wait(priv);
 }
 
-static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
+static void gve_adminq_get_create_rx_queue_cmd(struct gve_priv *priv,
+					       union gve_adminq_command *cmd,
+					       u32 queue_index)
 {
 	struct gve_rx_ring *rx = &priv->rx[queue_index];
-	union gve_adminq_command cmd;
 
-	memset(&cmd, 0, sizeof(cmd));
-	cmd.opcode = cpu_to_be32(GVE_ADMINQ_CREATE_RX_QUEUE);
-	cmd.create_rx_queue = (struct gve_adminq_create_rx_queue) {
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->opcode = cpu_to_be32(GVE_ADMINQ_CREATE_RX_QUEUE);
+	cmd->create_rx_queue = (struct gve_adminq_create_rx_queue) {
 		.queue_id = cpu_to_be32(queue_index),
 		.ntfy_id = cpu_to_be32(rx->ntfy_id),
 		.queue_resources_addr = cpu_to_be64(rx->q_resources_bus),
@@ -648,13 +649,13 @@  static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
 		u32 qpl_id = priv->queue_format == GVE_GQI_RDA_FORMAT ?
 			GVE_RAW_ADDRESSING_QPL_ID : rx->data.qpl->id;
 
-		cmd.create_rx_queue.rx_desc_ring_addr =
+		cmd->create_rx_queue.rx_desc_ring_addr =
 			cpu_to_be64(rx->desc.bus),
-		cmd.create_rx_queue.rx_data_ring_addr =
+		cmd->create_rx_queue.rx_data_ring_addr =
 			cpu_to_be64(rx->data.data_bus),
-		cmd.create_rx_queue.index = cpu_to_be32(queue_index);
-		cmd.create_rx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
-		cmd.create_rx_queue.packet_buffer_size = cpu_to_be16(rx->packet_buffer_size);
+		cmd->create_rx_queue.index = cpu_to_be32(queue_index);
+		cmd->create_rx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
+		cmd->create_rx_queue.packet_buffer_size = cpu_to_be16(rx->packet_buffer_size);
 	} else {
 		u32 qpl_id = 0;
 
@@ -662,25 +663,39 @@  static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
 			qpl_id = GVE_RAW_ADDRESSING_QPL_ID;
 		else
 			qpl_id = rx->dqo.qpl->id;
-		cmd.create_rx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
-		cmd.create_rx_queue.rx_desc_ring_addr =
+		cmd->create_rx_queue.queue_page_list_id = cpu_to_be32(qpl_id);
+		cmd->create_rx_queue.rx_desc_ring_addr =
 			cpu_to_be64(rx->dqo.complq.bus);
-		cmd.create_rx_queue.rx_data_ring_addr =
+		cmd->create_rx_queue.rx_data_ring_addr =
 			cpu_to_be64(rx->dqo.bufq.bus);
-		cmd.create_rx_queue.packet_buffer_size =
+		cmd->create_rx_queue.packet_buffer_size =
 			cpu_to_be16(priv->data_buffer_size_dqo);
-		cmd.create_rx_queue.rx_buff_ring_size =
+		cmd->create_rx_queue.rx_buff_ring_size =
 			cpu_to_be16(priv->rx_desc_cnt);
-		cmd.create_rx_queue.enable_rsc =
+		cmd->create_rx_queue.enable_rsc =
 			!!(priv->dev->features & NETIF_F_LRO);
 		if (priv->header_split_enabled)
-			cmd.create_rx_queue.header_buffer_size =
+			cmd->create_rx_queue.header_buffer_size =
 				cpu_to_be16(priv->header_buf_size);
 	}
+}
 
+static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
+{
+	union gve_adminq_command cmd;
+
+	gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
 	return gve_adminq_issue_cmd(priv, &cmd);
 }
 
+int gve_adminq_create_single_rx_queue(struct gve_priv *priv, u32 queue_index)
+{
+	union gve_adminq_command cmd;
+
+	gve_adminq_get_create_rx_queue_cmd(priv, &cmd, queue_index);
+	return gve_adminq_execute_cmd(priv, &cmd);
+}
+
 int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues)
 {
 	int err;
@@ -727,17 +742,22 @@  int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_qu
 	return gve_adminq_kick_and_wait(priv);
 }
 
+static void gve_adminq_make_destroy_rx_queue_cmd(union gve_adminq_command *cmd,
+						 u32 queue_index)
+{
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
+	cmd->destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
+		.queue_id = cpu_to_be32(queue_index),
+	};
+}
+
 static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
 {
 	union gve_adminq_command cmd;
 	int err;
 
-	memset(&cmd, 0, sizeof(cmd));
-	cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_RX_QUEUE);
-	cmd.destroy_rx_queue = (struct gve_adminq_destroy_rx_queue) {
-		.queue_id = cpu_to_be32(queue_index),
-	};
-
+	gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
 	err = gve_adminq_issue_cmd(priv, &cmd);
 	if (err)
 		return err;
@@ -745,6 +765,19 @@  static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
 	return 0;
 }
 
+int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index)
+{
+	union gve_adminq_command cmd;
+	int err;
+
+	gve_adminq_make_destroy_rx_queue_cmd(&cmd, queue_index);
+	err = gve_adminq_execute_cmd(priv, &cmd);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 int gve_adminq_destroy_rx_queues(struct gve_priv *priv, u32 num_queues)
 {
 	int err;
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
index beedf2353847..e64f0dbe744d 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.h
+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
@@ -451,7 +451,9 @@  int gve_adminq_configure_device_resources(struct gve_priv *priv,
 int gve_adminq_deconfigure_device_resources(struct gve_priv *priv);
 int gve_adminq_create_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_queues);
 int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_queues);
+int gve_adminq_create_single_rx_queue(struct gve_priv *priv, u32 queue_index);
 int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues);
+int gve_adminq_destroy_single_rx_queue(struct gve_priv *priv, u32 queue_index);
 int gve_adminq_destroy_rx_queues(struct gve_priv *priv, u32 queue_id);
 int gve_adminq_register_page_list(struct gve_priv *priv,
 				  struct gve_queue_page_list *qpl);