From: Simon Horman <horms@kernel.org>
To: Ziwei Xiao <ziweixiao@google.com>
Cc: netdev@vger.kernel.org, jeroendb@google.com,
pkaligineedi@google.com, shailend@google.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, willemb@google.com, hramamurthy@google.com,
rushilg@google.com, jfraker@google.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 5/5] gve: Add flow steering ethtool support
Date: Sat, 11 May 2024 15:45:32 +0100 [thread overview]
Message-ID: <20240511144532.GJ2347895@kernel.org> (raw)
In-Reply-To: <20240507225945.1408516-6-ziweixiao@google.com>
On Tue, May 07, 2024 at 10:59:45PM +0000, Ziwei Xiao wrote:
> From: Jeroen de Borst <jeroendb@google.com>
>
> Implement the ethtool commands that can be used to configure and query
> flow-steering rules. For these ethtool commands, the driver will
> temporarily drop the rtnl lock to reduce the latency for the flow
> steering commands on separate NICs. It will then be protected by the new
> added adminq lock.
>
> A large part of this change consists of translating the ethtool
> representation of 'ntuples' to our internal gve_flow_rule and vice-versa
> in the new created gve_flow_rule.c
>
> Considering the possible large amount of flow rules, the driver doesn't
> store all the rules locally. When the user runs 'ethtool -n <nic>' to
> check the registered rules, the driver will send adminq command to
> query a limited amount of rules/rule ids(that filled in a 4096 bytes dma
> memory) at a time as a cache for the ethtool queries. The adminq query
> commands will be repeated for several times until the ethtool has
> queried all the needed rules.
>
> Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> Co-developed-by: Ziwei Xiao <ziweixiao@google.com>
> Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
...
> diff --git a/drivers/net/ethernet/google/gve/gve_flow_rule.c b/drivers/net/ethernet/google/gve/gve_flow_rule.c
> new file mode 100644
> index 000000000000..1cafd520f2db
> --- /dev/null
> +++ b/drivers/net/ethernet/google/gve/gve_flow_rule.c
> @@ -0,0 +1,296 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/* Google virtual Ethernet (gve) driver
> + *
> + * Copyright (C) 2015-2024 Google LLC
> + */
> +
> +#include "gve.h"
> +#include "gve_adminq.h"
> +
> +static
> +int gve_fill_ethtool_flow_spec(struct ethtool_rx_flow_spec *fsp, struct gve_flow_rule *rule)
> +{
> + static const u16 flow_type_lut[] = {
> + [GVE_FLOW_TYPE_TCPV4] = TCP_V4_FLOW,
> + [GVE_FLOW_TYPE_UDPV4] = UDP_V4_FLOW,
> + [GVE_FLOW_TYPE_SCTPV4] = SCTP_V4_FLOW,
> + [GVE_FLOW_TYPE_AHV4] = AH_V4_FLOW,
> + [GVE_FLOW_TYPE_ESPV4] = ESP_V4_FLOW,
> + [GVE_FLOW_TYPE_TCPV6] = TCP_V6_FLOW,
> + [GVE_FLOW_TYPE_UDPV6] = UDP_V6_FLOW,
> + [GVE_FLOW_TYPE_SCTPV6] = SCTP_V6_FLOW,
> + [GVE_FLOW_TYPE_AHV6] = AH_V6_FLOW,
> + [GVE_FLOW_TYPE_ESPV6] = ESP_V6_FLOW,
> + };
> +
> + if (be16_to_cpu(rule->flow_type) >= ARRAY_SIZE(flow_type_lut))
The type of rule->flow_type is u16.
But be16_to_cpu expects a 16-bit big endian value as it's argument.
This does not seem right.
This was flagged by Sparse along with several other problems in this patch.
Please make sure patches don't introduce new Sparse warnings.
Thanks!
...
> +int gve_add_flow_rule(struct gve_priv *priv, struct ethtool_rxnfc *cmd)
> +{
> + struct ethtool_rx_flow_spec *fsp = &cmd->fs;
> + struct gve_adminq_flow_rule *rule = NULL;
> + int err;
> +
> + if (!priv->max_flow_rules)
> + return -EOPNOTSUPP;
> +
> + rule = kvzalloc(sizeof(*rule), GFP_KERNEL);
> + if (!rule)
> + return -ENOMEM;
> +
> + err = gve_generate_flow_rule(priv, fsp, rule);
> + if (err)
> + goto out;
> +
> + err = gve_adminq_add_flow_rule(priv, rule, fsp->location);
> +
> +out:
> + kfree(rule);
rule was allocated using kvmalloc(), so it should be freed using kvfree().
Flagged by Coccinelle.
> + if (err)
> + dev_err(&priv->pdev->dev, "Failed to add the flow rule: %u", fsp->location);
> +
> + return err;
> +}
...
prev parent reply other threads:[~2024-05-11 14:45 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 22:59 [PATCH net-next 0/5] gve: Add flow steering support Ziwei Xiao
2024-05-07 22:59 ` [PATCH net-next 1/5] gve: Add adminq mutex lock Ziwei Xiao
2024-05-08 12:07 ` Markus Elfring
2024-05-07 22:59 ` [PATCH net-next 2/5] gve: Add adminq extended command Ziwei Xiao
2024-05-08 12:17 ` Markus Elfring
2024-05-11 14:32 ` Simon Horman
2024-05-07 22:59 ` [PATCH net-next 3/5] gve: Add flow steering device option Ziwei Xiao
2024-05-08 5:32 ` David Wei
2024-05-10 0:18 ` Ziwei Xiao
2024-05-08 5:34 ` [PATCH net-next 2/5] gve: Add adminq extended command David Wei
2024-05-10 0:17 ` Ziwei Xiao
2024-05-07 22:59 ` [PATCH net-next 4/5] gve: Add flow steering adminq commands Ziwei Xiao
2024-05-08 6:24 ` David Wei
2024-05-10 0:18 ` Ziwei Xiao
2024-05-20 17:27 ` David Wei
2024-05-08 13:23 ` Markus Elfring
2024-05-08 14:24 ` kernel test robot
2024-05-07 22:59 ` [PATCH net-next 5/5] gve: Add flow steering ethtool support Ziwei Xiao
2024-05-08 14:09 ` Markus Elfring
2024-05-10 0:19 ` Ziwei Xiao
2024-05-10 3:16 ` Jakub Kicinski
2024-05-10 6:45 ` Markus Elfring
2024-05-11 14:45 ` Simon Horman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240511144532.GJ2347895@kernel.org \
--to=horms@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hramamurthy@google.com \
--cc=jeroendb@google.com \
--cc=jfraker@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pkaligineedi@google.com \
--cc=rushilg@google.com \
--cc=shailend@google.com \
--cc=willemb@google.com \
--cc=ziweixiao@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).