* [PATCH net-next v3 0/2] gve: Handle alternate miss-completions
@ 2022-11-14 23:35 Jeroen de Borst
2022-11-14 23:35 ` [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver Jeroen de Borst
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jeroen de Borst @ 2022-11-14 23:35 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jesse.brandeburg, Jeroen de Borst
Some versions of the virtual NIC present miss-completions in
an alternative way. Let the diver handle these alternate completions
and announce this capability to the device.
Changed in v3:
- Rewording cover letter
- Added 'Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>'
Changes in v2:
- Changed the subject to include 'gve:'
Jeroen de Borst (2):
gve: Adding a new AdminQ command to verify driver
gve: Handle alternate miss completions
drivers/net/ethernet/google/gve/gve.h | 1 +
drivers/net/ethernet/google/gve/gve_adminq.c | 19 +++++++
drivers/net/ethernet/google/gve/gve_adminq.h | 51 ++++++++++++++++++
.../net/ethernet/google/gve/gve_desc_dqo.h | 5 ++
drivers/net/ethernet/google/gve/gve_main.c | 52 +++++++++++++++++++
drivers/net/ethernet/google/gve/gve_tx_dqo.c | 18 ++++---
6 files changed, 140 insertions(+), 6 deletions(-)
--
2.38.1.431.g37b22c650d-goog
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver 2022-11-14 23:35 [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Jeroen de Borst @ 2022-11-14 23:35 ` Jeroen de Borst 2022-11-16 5:34 ` Saeed Mahameed 2022-11-14 23:35 ` [PATCH net-next v3 2/2] gve: Handle alternate miss completions Jeroen de Borst 2022-11-16 5:38 ` [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Saeed Mahameed 2 siblings, 1 reply; 8+ messages in thread From: Jeroen de Borst @ 2022-11-14 23:35 UTC (permalink / raw) To: netdev; +Cc: davem, kuba, jesse.brandeburg, Jeroen de Borst Check whether the driver is compatible with the device presented. Signed-off-by: Jeroen de Borst <jeroendb@google.com> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> --- drivers/net/ethernet/google/gve/gve.h | 1 + drivers/net/ethernet/google/gve/gve_adminq.c | 19 +++++++ drivers/net/ethernet/google/gve/gve_adminq.h | 49 ++++++++++++++++++ drivers/net/ethernet/google/gve/gve_main.c | 52 ++++++++++++++++++++ 4 files changed, 121 insertions(+) diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h index 5655da9cd236..64eb0442c82f 100644 --- a/drivers/net/ethernet/google/gve/gve.h +++ b/drivers/net/ethernet/google/gve/gve.h @@ -563,6 +563,7 @@ struct gve_priv { u32 adminq_report_stats_cnt; u32 adminq_report_link_speed_cnt; u32 adminq_get_ptype_map_cnt; + u32 adminq_verify_driver_compatibility_cnt; /* Global stats */ u32 interface_up_cnt; /* count of times interface turned up since last reset */ diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c index f7621ab672b9..6a12b30a9f87 100644 --- a/drivers/net/ethernet/google/gve/gve_adminq.c +++ b/drivers/net/ethernet/google/gve/gve_adminq.c @@ -407,6 +407,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv, case GVE_ADMINQ_GET_PTYPE_MAP: priv->adminq_get_ptype_map_cnt++; break; + case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY: + priv->adminq_verify_driver_compatibility_cnt++; + break; default: dev_err(&priv->pdev->dev, "unknown AQ command opcode %d\n", opcode); } @@ -878,6 +881,22 @@ int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len, return gve_adminq_execute_cmd(priv, &cmd); } +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, + u64 driver_info_len, + dma_addr_t driver_info_addr) +{ + union gve_adminq_command cmd; + + memset(&cmd, 0, sizeof(cmd)); + cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY); + cmd.verify_driver_compatibility = (struct gve_adminq_verify_driver_compatibility) { + .driver_info_len = cpu_to_be64(driver_info_len), + .driver_info_addr = cpu_to_be64(driver_info_addr), + }; + + return gve_adminq_execute_cmd(priv, &cmd); +} + int gve_adminq_report_link_speed(struct gve_priv *priv) { union gve_adminq_command gvnic_cmd; diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h index 83c0b40cd2d9..b9ee8be73f96 100644 --- a/drivers/net/ethernet/google/gve/gve_adminq.h +++ b/drivers/net/ethernet/google/gve/gve_adminq.h @@ -24,6 +24,7 @@ enum gve_adminq_opcodes { GVE_ADMINQ_REPORT_STATS = 0xC, GVE_ADMINQ_REPORT_LINK_SPEED = 0xD, GVE_ADMINQ_GET_PTYPE_MAP = 0xE, + GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY = 0xF, }; /* Admin queue status codes */ @@ -146,6 +147,49 @@ enum gve_sup_feature_mask { #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0 +#define GVE_VERSION_STR_LEN 128 + +enum gve_driver_capbility { + gve_driver_capability_gqi_qpl = 0, + gve_driver_capability_gqi_rda = 1, + gve_driver_capability_dqo_qpl = 2, /* reserved for future use */ + gve_driver_capability_dqo_rda = 3, +}; + +#define GVE_CAP1(a) BIT((int)a) +#define GVE_CAP2(a) BIT(((int)a) - 64) +#define GVE_CAP3(a) BIT(((int)a) - 128) +#define GVE_CAP4(a) BIT(((int)a) - 192) + +#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)) + +#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0 +#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0 +#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0 + +struct gve_driver_info { + u8 os_type; /* 0x01 = Linux */ + u8 driver_major; + u8 driver_minor; + u8 driver_sub; + __be32 os_version_major; + __be32 os_version_minor; + __be32 os_version_sub; + __be64 driver_capability_flags[4]; + u8 os_version_str1[GVE_VERSION_STR_LEN]; + u8 os_version_str2[GVE_VERSION_STR_LEN]; +}; + +struct gve_adminq_verify_driver_compatibility { + __be64 driver_info_len; + __be64 driver_info_addr; +}; + +static_assert(sizeof(struct gve_adminq_verify_driver_compatibility) == 16); + struct gve_adminq_configure_device_resources { __be64 counter_array; __be64 irq_db_addr; @@ -345,6 +389,8 @@ union gve_adminq_command { struct gve_adminq_report_stats report_stats; struct gve_adminq_report_link_speed report_link_speed; struct gve_adminq_get_ptype_map get_ptype_map; + struct gve_adminq_verify_driver_compatibility + verify_driver_compatibility; }; }; u8 reserved[64]; @@ -372,6 +418,9 @@ int gve_adminq_unregister_page_list(struct gve_priv *priv, u32 page_list_id); int gve_adminq_set_mtu(struct gve_priv *priv, u64 mtu); int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len, dma_addr_t stats_report_addr, u64 interval); +int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, + u64 driver_info_len, + dma_addr_t driver_info_addr); int gve_adminq_report_link_speed(struct gve_priv *priv); struct gve_ptype_lut; diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c index 5a229a01f49d..5b40f9c53196 100644 --- a/drivers/net/ethernet/google/gve/gve_main.c +++ b/drivers/net/ethernet/google/gve/gve_main.c @@ -12,6 +12,8 @@ #include <linux/sched.h> #include <linux/timer.h> #include <linux/workqueue.h> +#include <linux/utsname.h> +#include <linux/version.h> #include <net/sch_generic.h> #include "gve.h" #include "gve_dqo.h" @@ -30,6 +32,49 @@ const char gve_version_str[] = GVE_VERSION; static const char gve_version_prefix[] = GVE_VERSION_PREFIX; +static int gve_verify_driver_compatibility(struct gve_priv *priv) +{ + int err; + struct gve_driver_info *driver_info; + dma_addr_t driver_info_bus; + + driver_info = dma_alloc_coherent(&priv->pdev->dev, + sizeof(struct gve_driver_info), + &driver_info_bus, GFP_KERNEL); + if (!driver_info) + return -ENOMEM; + + *driver_info = (struct gve_driver_info) { + .os_type = 1, /* Linux */ + .os_version_major = cpu_to_be32(LINUX_VERSION_MAJOR), + .os_version_minor = cpu_to_be32(LINUX_VERSION_SUBLEVEL), + .os_version_sub = cpu_to_be32(LINUX_VERSION_PATCHLEVEL), + .driver_capability_flags = { + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1), + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2), + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3), + cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4), + }, + }; + strscpy(driver_info->os_version_str1, utsname()->release, + sizeof(driver_info->os_version_str1)); + strscpy(driver_info->os_version_str2, utsname()->version, + sizeof(driver_info->os_version_str2)); + + err = gve_adminq_verify_driver_compatibility(priv, + sizeof(struct gve_driver_info), + driver_info_bus); + + /* It's ok if the device doesn't support this */ + if (err == -EOPNOTSUPP) + err = 0; + + dma_free_coherent(&priv->pdev->dev, + sizeof(struct gve_driver_info), + driver_info, driver_info_bus); + return err; +} + static netdev_tx_t gve_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct gve_priv *priv = netdev_priv(dev); @@ -1368,6 +1413,13 @@ static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device) return err; } + err = gve_verify_driver_compatibility(priv); + if (err) { + dev_err(&priv->pdev->dev, + "Could not verify driver compatibility: err=%d\n", err); + goto err; + } + if (skip_describe_device) goto setup_device; -- 2.38.1.431.g37b22c650d-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver 2022-11-14 23:35 ` [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver Jeroen de Borst @ 2022-11-16 5:34 ` Saeed Mahameed 0 siblings, 0 replies; 8+ messages in thread From: Saeed Mahameed @ 2022-11-16 5:34 UTC (permalink / raw) To: Jeroen de Borst; +Cc: netdev, davem, kuba, jesse.brandeburg On 14 Nov 15:35, Jeroen de Borst wrote: >Check whether the driver is compatible with the device >presented. > >Signed-off-by: Jeroen de Borst <jeroendb@google.com> >Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> >--- > drivers/net/ethernet/google/gve/gve.h | 1 + > drivers/net/ethernet/google/gve/gve_adminq.c | 19 +++++++ > drivers/net/ethernet/google/gve/gve_adminq.h | 49 ++++++++++++++++++ > drivers/net/ethernet/google/gve/gve_main.c | 52 ++++++++++++++++++++ > 4 files changed, 121 insertions(+) > >diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h >index 5655da9cd236..64eb0442c82f 100644 >--- a/drivers/net/ethernet/google/gve/gve.h >+++ b/drivers/net/ethernet/google/gve/gve.h >@@ -563,6 +563,7 @@ struct gve_priv { > u32 adminq_report_stats_cnt; > u32 adminq_report_link_speed_cnt; > u32 adminq_get_ptype_map_cnt; >+ u32 adminq_verify_driver_compatibility_cnt; > > /* Global stats */ > u32 interface_up_cnt; /* count of times interface turned up since last reset */ >diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c >index f7621ab672b9..6a12b30a9f87 100644 >--- a/drivers/net/ethernet/google/gve/gve_adminq.c >+++ b/drivers/net/ethernet/google/gve/gve_adminq.c >@@ -407,6 +407,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv, > case GVE_ADMINQ_GET_PTYPE_MAP: > priv->adminq_get_ptype_map_cnt++; > break; >+ case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY: >+ priv->adminq_verify_driver_compatibility_cnt++; >+ break; > default: > dev_err(&priv->pdev->dev, "unknown AQ command opcode %d\n", opcode); > } >@@ -878,6 +881,22 @@ int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len, > return gve_adminq_execute_cmd(priv, &cmd); > } > >+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, >+ u64 driver_info_len, >+ dma_addr_t driver_info_addr) >+{ >+ union gve_adminq_command cmd; >+ >+ memset(&cmd, 0, sizeof(cmd)); >+ cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY); >+ cmd.verify_driver_compatibility = (struct gve_adminq_verify_driver_compatibility) { >+ .driver_info_len = cpu_to_be64(driver_info_len), >+ .driver_info_addr = cpu_to_be64(driver_info_addr), >+ }; >+ >+ return gve_adminq_execute_cmd(priv, &cmd); >+} >+ > int gve_adminq_report_link_speed(struct gve_priv *priv) > { > union gve_adminq_command gvnic_cmd; >diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h >index 83c0b40cd2d9..b9ee8be73f96 100644 >--- a/drivers/net/ethernet/google/gve/gve_adminq.h >+++ b/drivers/net/ethernet/google/gve/gve_adminq.h >@@ -24,6 +24,7 @@ enum gve_adminq_opcodes { > GVE_ADMINQ_REPORT_STATS = 0xC, > GVE_ADMINQ_REPORT_LINK_SPEED = 0xD, > GVE_ADMINQ_GET_PTYPE_MAP = 0xE, >+ GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY = 0xF, > }; > > /* Admin queue status codes */ >@@ -146,6 +147,49 @@ enum gve_sup_feature_mask { > > #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0 > >+#define GVE_VERSION_STR_LEN 128 >+ >+enum gve_driver_capbility { >+ gve_driver_capability_gqi_qpl = 0, >+ gve_driver_capability_gqi_rda = 1, >+ gve_driver_capability_dqo_qpl = 2, /* reserved for future use */ >+ gve_driver_capability_dqo_rda = 3, >+}; >+ >+#define GVE_CAP1(a) BIT((int)a) >+#define GVE_CAP2(a) BIT(((int)a) - 64) >+#define GVE_CAP3(a) BIT(((int)a) - 128) >+#define GVE_CAP4(a) BIT(((int)a) - 192) >+ >+#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)) >+ >+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0 >+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0 >+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0 >+ >+struct gve_driver_info { >+ u8 os_type; /* 0x01 = Linux */ >+ u8 driver_major; >+ u8 driver_minor; >+ u8 driver_sub; >+ __be32 os_version_major; >+ __be32 os_version_minor; >+ __be32 os_version_sub; >+ __be64 driver_capability_flags[4]; >+ u8 os_version_str1[GVE_VERSION_STR_LEN]; >+ u8 os_version_str2[GVE_VERSION_STR_LEN]; >+}; >+ >+struct gve_adminq_verify_driver_compatibility { >+ __be64 driver_info_len; >+ __be64 driver_info_addr; >+}; >+ >+static_assert(sizeof(struct gve_adminq_verify_driver_compatibility) == 16); >+ > struct gve_adminq_configure_device_resources { > __be64 counter_array; > __be64 irq_db_addr; >@@ -345,6 +389,8 @@ union gve_adminq_command { > struct gve_adminq_report_stats report_stats; > struct gve_adminq_report_link_speed report_link_speed; > struct gve_adminq_get_ptype_map get_ptype_map; >+ struct gve_adminq_verify_driver_compatibility >+ verify_driver_compatibility; > }; > }; > u8 reserved[64]; >@@ -372,6 +418,9 @@ int gve_adminq_unregister_page_list(struct gve_priv *priv, u32 page_list_id); > int gve_adminq_set_mtu(struct gve_priv *priv, u64 mtu); > int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len, > dma_addr_t stats_report_addr, u64 interval); >+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv, >+ u64 driver_info_len, >+ dma_addr_t driver_info_addr); > int gve_adminq_report_link_speed(struct gve_priv *priv); > > struct gve_ptype_lut; >diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c >index 5a229a01f49d..5b40f9c53196 100644 >--- a/drivers/net/ethernet/google/gve/gve_main.c >+++ b/drivers/net/ethernet/google/gve/gve_main.c >@@ -12,6 +12,8 @@ > #include <linux/sched.h> > #include <linux/timer.h> > #include <linux/workqueue.h> >+#include <linux/utsname.h> >+#include <linux/version.h> > #include <net/sch_generic.h> > #include "gve.h" > #include "gve_dqo.h" >@@ -30,6 +32,49 @@ > const char gve_version_str[] = GVE_VERSION; > static const char gve_version_prefix[] = GVE_VERSION_PREFIX; > >+static int gve_verify_driver_compatibility(struct gve_priv *priv) >+{ >+ int err; >+ struct gve_driver_info *driver_info; >+ dma_addr_t driver_info_bus; >+ >+ driver_info = dma_alloc_coherent(&priv->pdev->dev, >+ sizeof(struct gve_driver_info), >+ &driver_info_bus, GFP_KERNEL); >+ if (!driver_info) >+ return -ENOMEM; >+ >+ *driver_info = (struct gve_driver_info) { >+ .os_type = 1, /* Linux */ >+ .os_version_major = cpu_to_be32(LINUX_VERSION_MAJOR), >+ .os_version_minor = cpu_to_be32(LINUX_VERSION_SUBLEVEL), >+ .os_version_sub = cpu_to_be32(LINUX_VERSION_PATCHLEVEL), >+ .driver_capability_flags = { >+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1), >+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2), >+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3), >+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4), >+ }, >+ }; >+ strscpy(driver_info->os_version_str1, utsname()->release, >+ sizeof(driver_info->os_version_str1)); >+ strscpy(driver_info->os_version_str2, utsname()->version, >+ sizeof(driver_info->os_version_str2)); >+ >+ err = gve_adminq_verify_driver_compatibility(priv, >+ sizeof(struct gve_driver_info), >+ driver_info_bus); >+ >+ /* It's ok if the device doesn't support this */ >+ if (err == -EOPNOTSUPP) >+ err = 0; >+ I always find these exception interesting so i had to attempt and chase this error code down to the source and couldn't find it. I think you meant -ENOTSUPP: which comes from gve_adminq_parse_err() case GVE_ADMINQ_COMMAND_ERROR_UNIMPLEMENTED: return -ENOTSUPP; because there isn't any path in the code that returns -EOPNOTSUPP; but isn't there any capability that could tell if driver version compatibility command is supported by device, prior to issuing the command, so the code can be more explicit. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v3 2/2] gve: Handle alternate miss completions 2022-11-14 23:35 [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Jeroen de Borst 2022-11-14 23:35 ` [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver Jeroen de Borst @ 2022-11-14 23:35 ` Jeroen de Borst 2022-11-16 5:17 ` Saeed Mahameed 2022-11-16 5:38 ` [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Saeed Mahameed 2 siblings, 1 reply; 8+ messages in thread From: Jeroen de Borst @ 2022-11-14 23:35 UTC (permalink / raw) To: netdev; +Cc: davem, kuba, jesse.brandeburg, Jeroen de Borst 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; + 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 2/2] gve: Handle alternate miss completions 2022-11-14 23:35 ` [PATCH net-next v3 2/2] gve: Handle alternate miss completions Jeroen de Borst @ 2022-11-16 5:17 ` Saeed Mahameed 2022-11-16 16:23 ` Jeroen de Borst 0 siblings, 1 reply; 8+ messages in thread From: Saeed Mahameed @ 2022-11-16 5:17 UTC (permalink / raw) To: Jeroen de Borst; +Cc: netdev, davem, kuba, jesse.brandeburg 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 2/2] gve: Handle alternate miss completions 2022-11-16 5:17 ` Saeed Mahameed @ 2022-11-16 16:23 ` Jeroen de Borst 2022-11-16 23:36 ` Saeed Mahameed 0 siblings, 1 reply; 8+ messages in thread From: Jeroen de Borst @ 2022-11-16 16:23 UTC (permalink / raw) To: Saeed Mahameed; +Cc: netdev, davem, kuba, jesse.brandeburg 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 2/2] gve: Handle alternate miss completions 2022-11-16 16:23 ` Jeroen de Borst @ 2022-11-16 23:36 ` Saeed Mahameed 0 siblings, 0 replies; 8+ messages in thread From: Saeed Mahameed @ 2022-11-16 23:36 UTC (permalink / raw) To: Jeroen de Borst; +Cc: netdev, davem, kuba, jesse.brandeburg 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v3 0/2] gve: Handle alternate miss-completions 2022-11-14 23:35 [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Jeroen de Borst 2022-11-14 23:35 ` [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver Jeroen de Borst 2022-11-14 23:35 ` [PATCH net-next v3 2/2] gve: Handle alternate miss completions Jeroen de Borst @ 2022-11-16 5:38 ` Saeed Mahameed 2 siblings, 0 replies; 8+ messages in thread From: Saeed Mahameed @ 2022-11-16 5:38 UTC (permalink / raw) To: Jeroen de Borst; +Cc: netdev, davem, kuba, jesse.brandeburg On 14 Nov 15:35, Jeroen de Borst wrote: >Some versions of the virtual NIC present miss-completions in >an alternative way. Let the diver handle these alternate completions >and announce this capability to the device. > nit: you missed to document the 1st "new AdminQ command" patch which has some special logic to validate driver/os version compatibility with the device and if it's related to this cover-letter title if at all. >Changed in v3: >- Rewording cover letter >- Added 'Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>' >Changes in v2: >- Changed the subject to include 'gve:' > > >Jeroen de Borst (2): > gve: Adding a new AdminQ command to verify driver > gve: Handle alternate miss completions > > drivers/net/ethernet/google/gve/gve.h | 1 + > drivers/net/ethernet/google/gve/gve_adminq.c | 19 +++++++ > drivers/net/ethernet/google/gve/gve_adminq.h | 51 ++++++++++++++++++ > .../net/ethernet/google/gve/gve_desc_dqo.h | 5 ++ > drivers/net/ethernet/google/gve/gve_main.c | 52 +++++++++++++++++++ > drivers/net/ethernet/google/gve/gve_tx_dqo.c | 18 ++++--- > 6 files changed, 140 insertions(+), 6 deletions(-) > >-- >2.38.1.431.g37b22c650d-goog > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-16 23:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-14 23:35 [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Jeroen de Borst 2022-11-14 23:35 ` [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to verify driver Jeroen de Borst 2022-11-16 5:34 ` Saeed Mahameed 2022-11-14 23:35 ` [PATCH net-next v3 2/2] gve: Handle alternate miss completions Jeroen de Borst 2022-11-16 5:17 ` Saeed Mahameed 2022-11-16 16:23 ` Jeroen de Borst 2022-11-16 23:36 ` Saeed Mahameed 2022-11-16 5:38 ` [PATCH net-next v3 0/2] gve: Handle alternate miss-completions Saeed Mahameed
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).