diff mbox series

[net-next,v3,2/2] gve: Handle alternate miss completions

Message ID 20221114233514.1913116-3-jeroendb@google.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series gve: Handle alternate miss-completions | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 5 maintainers not CCed: pabeni@redhat.com yuanjilin@cdjrlc.com edumazet@google.com shailend@google.com csully@google.com
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
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: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jeroen de Borst Nov. 14, 2022, 11:35 p.m. UTC
The virtual NIC has 2 ways of indicating a miss-path
completion. This handles the alternate.

Signed-off-by: Jeroen de Borst <jeroendb@google.com>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/google/gve/gve_adminq.h   |  4 +++-
 drivers/net/ethernet/google/gve/gve_desc_dqo.h |  5 +++++
 drivers/net/ethernet/google/gve/gve_tx_dqo.c   | 18 ++++++++++++------
 3 files changed, 20 insertions(+), 7 deletions(-)

Comments

Saeed Mahameed Nov. 16, 2022, 5:17 a.m. UTC | #1
On 14 Nov 15:35, Jeroen de Borst wrote:
>The virtual NIC has 2 ways of indicating a miss-path
>completion. This handles the alternate.
>
>Signed-off-by: Jeroen de Borst <jeroendb@google.com>
>Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>---
> drivers/net/ethernet/google/gve/gve_adminq.h   |  4 +++-
> drivers/net/ethernet/google/gve/gve_desc_dqo.h |  5 +++++
> drivers/net/ethernet/google/gve/gve_tx_dqo.c   | 18 ++++++++++++------
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
>index b9ee8be73f96..cf29662e6ad1 100644
>--- a/drivers/net/ethernet/google/gve/gve_adminq.h
>+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
>@@ -154,6 +154,7 @@ enum gve_driver_capbility {
> 	gve_driver_capability_gqi_rda = 1,
> 	gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
> 	gve_driver_capability_dqo_rda = 3,
>+	gve_driver_capability_alt_miss_compl = 4,
> };
>
> #define GVE_CAP1(a) BIT((int)a)
>@@ -164,7 +165,8 @@ enum gve_driver_capbility {
> #define GVE_DRIVER_CAPABILITY_FLAGS1 \
> 	(GVE_CAP1(gve_driver_capability_gqi_qpl) | \
> 	 GVE_CAP1(gve_driver_capability_gqi_rda) | \
>-	 GVE_CAP1(gve_driver_capability_dqo_rda))
>+	 GVE_CAP1(gve_driver_capability_dqo_rda) | \
>+	 GVE_CAP1(gve_driver_capability_alt_miss_compl))
>
> #define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
> #define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
>diff --git a/drivers/net/ethernet/google/gve/gve_desc_dqo.h b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
>index e8fe9adef7f2..f79cd0591110 100644
>--- a/drivers/net/ethernet/google/gve/gve_desc_dqo.h
>+++ b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
>@@ -176,6 +176,11 @@ static_assert(sizeof(struct gve_tx_compl_desc) == 8);
> #define GVE_COMPL_TYPE_DQO_MISS 0x1 /* Miss path completion */
> #define GVE_COMPL_TYPE_DQO_REINJECTION 0x3 /* Re-injection completion */
>
>+/* The most significant bit in the completion tag can change the completion
>+ * type from packet completion to miss path completion.
>+ */
>+#define GVE_ALT_MISS_COMPL_BIT BIT(15)
>+
> /* Descriptor to post buffers to HW on buffer queue. */
> struct gve_rx_desc_dqo {
> 	__le16 buf_id; /* ID returned in Rx completion descriptor */
>diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
>index 588d64819ed5..762915c6063b 100644
>--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
>+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
>@@ -953,12 +953,18 @@ int gve_clean_tx_done_dqo(struct gve_priv *priv, struct gve_tx_ring *tx,
> 			atomic_set_release(&tx->dqo_compl.hw_tx_head, tx_head);
> 		} else if (type == GVE_COMPL_TYPE_DQO_PKT) {
> 			u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);
>-
>-			gve_handle_packet_completion(priv, tx, !!napi,
>-						     compl_tag,
>-						     &pkt_compl_bytes,
>-						     &pkt_compl_pkts,
>-						     /*is_reinjection=*/false);
>+			if (compl_tag & GVE_ALT_MISS_COMPL_BIT) {
>+				compl_tag &= ~GVE_ALT_MISS_COMPL_BIT;

nit: __test_and_clear_bit() and reduce to oneline. also you can drop the
braces in the if else statements once you squashed the two lines.

>+				gve_handle_miss_completion(priv, tx, compl_tag,
>+							   &miss_compl_bytes,
>+							   &miss_compl_pkts);
>+			} else {
>+				gve_handle_packet_completion(priv, tx, !!napi,
>+							     compl_tag,
>+							     &pkt_compl_bytes,
>+							     &pkt_compl_pkts,
>+							     /*is_reinjection=*/false);
>+			}
> 		} else if (type == GVE_COMPL_TYPE_DQO_MISS) {
> 			u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);
>
>-- 
>2.38.1.431.g37b22c650d-goog
>
Jeroen de Borst Nov. 16, 2022, 4:23 p.m. UTC | #2
Saeed,

Thanks for your review. I like the suggestion, but
__test_and_clear_bit is for unsigned longs, comptag is a short.

Jeroen


On Tue, Nov 15, 2022 at 9:17 PM Saeed Mahameed <saeed@kernel.org> wrote:
>
> On 14 Nov 15:35, Jeroen de Borst wrote:
> >The virtual NIC has 2 ways of indicating a miss-path
> >completion. This handles the alternate.
> >
> >Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> >Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >---
> > drivers/net/ethernet/google/gve/gve_adminq.h   |  4 +++-
> > drivers/net/ethernet/google/gve/gve_desc_dqo.h |  5 +++++
> > drivers/net/ethernet/google/gve/gve_tx_dqo.c   | 18 ++++++++++++------
> > 3 files changed, 20 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
> >index b9ee8be73f96..cf29662e6ad1 100644
> >--- a/drivers/net/ethernet/google/gve/gve_adminq.h
> >+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
> >@@ -154,6 +154,7 @@ enum gve_driver_capbility {
> >       gve_driver_capability_gqi_rda = 1,
> >       gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
> >       gve_driver_capability_dqo_rda = 3,
> >+      gve_driver_capability_alt_miss_compl = 4,
> > };
> >
> > #define GVE_CAP1(a) BIT((int)a)
> >@@ -164,7 +165,8 @@ enum gve_driver_capbility {
> > #define GVE_DRIVER_CAPABILITY_FLAGS1 \
> >       (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
> >        GVE_CAP1(gve_driver_capability_gqi_rda) | \
> >-       GVE_CAP1(gve_driver_capability_dqo_rda))
> >+       GVE_CAP1(gve_driver_capability_dqo_rda) | \
> >+       GVE_CAP1(gve_driver_capability_alt_miss_compl))
> >
> > #define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
> > #define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
> >diff --git a/drivers/net/ethernet/google/gve/gve_desc_dqo.h b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
> >index e8fe9adef7f2..f79cd0591110 100644
> >--- a/drivers/net/ethernet/google/gve/gve_desc_dqo.h
> >+++ b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
> >@@ -176,6 +176,11 @@ static_assert(sizeof(struct gve_tx_compl_desc) == 8);
> > #define GVE_COMPL_TYPE_DQO_MISS 0x1 /* Miss path completion */
> > #define GVE_COMPL_TYPE_DQO_REINJECTION 0x3 /* Re-injection completion */
> >
> >+/* The most significant bit in the completion tag can change the completion
> >+ * type from packet completion to miss path completion.
> >+ */
> >+#define GVE_ALT_MISS_COMPL_BIT BIT(15)
> >+
> > /* Descriptor to post buffers to HW on buffer queue. */
> > struct gve_rx_desc_dqo {
> >       __le16 buf_id; /* ID returned in Rx completion descriptor */
> >diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
> >index 588d64819ed5..762915c6063b 100644
> >--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
> >+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
> >@@ -953,12 +953,18 @@ int gve_clean_tx_done_dqo(struct gve_priv *priv, struct gve_tx_ring *tx,
> >                       atomic_set_release(&tx->dqo_compl.hw_tx_head, tx_head);
> >               } else if (type == GVE_COMPL_TYPE_DQO_PKT) {
> >                       u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);
> >-
> >-                      gve_handle_packet_completion(priv, tx, !!napi,
> >-                                                   compl_tag,
> >-                                                   &pkt_compl_bytes,
> >-                                                   &pkt_compl_pkts,
> >-                                                   /*is_reinjection=*/false);
> >+                      if (compl_tag & GVE_ALT_MISS_COMPL_BIT) {
> >+                              compl_tag &= ~GVE_ALT_MISS_COMPL_BIT;
>
> nit: __test_and_clear_bit() and reduce to oneline. also you can drop the
> braces in the if else statements once you squashed the two lines.
>
> >+                              gve_handle_miss_completion(priv, tx, compl_tag,
> >+                                                         &miss_compl_bytes,
> >+                                                         &miss_compl_pkts);
> >+                      } else {
> >+                              gve_handle_packet_completion(priv, tx, !!napi,
> >+                                                           compl_tag,
> >+                                                           &pkt_compl_bytes,
> >+                                                           &pkt_compl_pkts,
> >+                                                           /*is_reinjection=*/false);
> >+                      }
> >               } else if (type == GVE_COMPL_TYPE_DQO_MISS) {
> >                       u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);
> >
> >--
> >2.38.1.431.g37b22c650d-goog
> >
Saeed Mahameed Nov. 16, 2022, 11:36 p.m. UTC | #3
On 16 Nov 08:23, Jeroen de Borst wrote:
>Saeed,
>
>Thanks for your review. I like the suggestion, but
>__test_and_clear_bit is for unsigned longs, comptag is a short.
>

gcc will up-cast it for you with no problem.
Assuming it's unsigned short.. if not then consider changing it to unsigned
short, or do the casting yourself.

anyway this was just a minor suggestion.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
index b9ee8be73f96..cf29662e6ad1 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.h
+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
@@ -154,6 +154,7 @@  enum gve_driver_capbility {
 	gve_driver_capability_gqi_rda = 1,
 	gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
 	gve_driver_capability_dqo_rda = 3,
+	gve_driver_capability_alt_miss_compl = 4,
 };
 
 #define GVE_CAP1(a) BIT((int)a)
@@ -164,7 +165,8 @@  enum gve_driver_capbility {
 #define GVE_DRIVER_CAPABILITY_FLAGS1 \
 	(GVE_CAP1(gve_driver_capability_gqi_qpl) | \
 	 GVE_CAP1(gve_driver_capability_gqi_rda) | \
-	 GVE_CAP1(gve_driver_capability_dqo_rda))
+	 GVE_CAP1(gve_driver_capability_dqo_rda) | \
+	 GVE_CAP1(gve_driver_capability_alt_miss_compl))
 
 #define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
 #define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
diff --git a/drivers/net/ethernet/google/gve/gve_desc_dqo.h b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
index e8fe9adef7f2..f79cd0591110 100644
--- a/drivers/net/ethernet/google/gve/gve_desc_dqo.h
+++ b/drivers/net/ethernet/google/gve/gve_desc_dqo.h
@@ -176,6 +176,11 @@  static_assert(sizeof(struct gve_tx_compl_desc) == 8);
 #define GVE_COMPL_TYPE_DQO_MISS 0x1 /* Miss path completion */
 #define GVE_COMPL_TYPE_DQO_REINJECTION 0x3 /* Re-injection completion */
 
+/* The most significant bit in the completion tag can change the completion
+ * type from packet completion to miss path completion.
+ */
+#define GVE_ALT_MISS_COMPL_BIT BIT(15)
+
 /* Descriptor to post buffers to HW on buffer queue. */
 struct gve_rx_desc_dqo {
 	__le16 buf_id; /* ID returned in Rx completion descriptor */
diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
index 588d64819ed5..762915c6063b 100644
--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
@@ -953,12 +953,18 @@  int gve_clean_tx_done_dqo(struct gve_priv *priv, struct gve_tx_ring *tx,
 			atomic_set_release(&tx->dqo_compl.hw_tx_head, tx_head);
 		} else if (type == GVE_COMPL_TYPE_DQO_PKT) {
 			u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);
-
-			gve_handle_packet_completion(priv, tx, !!napi,
-						     compl_tag,
-						     &pkt_compl_bytes,
-						     &pkt_compl_pkts,
-						     /*is_reinjection=*/false);
+			if (compl_tag & GVE_ALT_MISS_COMPL_BIT) {
+				compl_tag &= ~GVE_ALT_MISS_COMPL_BIT;
+				gve_handle_miss_completion(priv, tx, compl_tag,
+							   &miss_compl_bytes,
+							   &miss_compl_pkts);
+			} else {
+				gve_handle_packet_completion(priv, tx, !!napi,
+							     compl_tag,
+							     &pkt_compl_bytes,
+							     &pkt_compl_pkts,
+							     /*is_reinjection=*/false);
+			}
 		} else if (type == GVE_COMPL_TYPE_DQO_MISS) {
 			u16 compl_tag = le16_to_cpu(compl_desc->completion_tag);