From: Stephen Hemminger <stephen@networkplumber.org>
To: Muhammad Jawad Hussain <jawad.hussain@emumba.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] examples/ethtool: update rxmode to increase functionality
Date: Mon, 3 Jul 2023 15:19:44 -0700 [thread overview]
Message-ID: <20230703151944.62af33ab@hermes.local> (raw)
In-Reply-To: <20220811125840.108887-1-jawad.hussain@emumba.com>
On Thu, 11 Aug 2022 17:58:40 +0500
Muhammad Jawad Hussain <jawad.hussain@emumba.com> wrote:
> @@ -142,7 +183,6 @@ pcmd_quit_callback(__rte_unused void *ptr_params,
> cmdline_quit(ctx);
> }
>
> -
> static void
> pcmd_drvinfo_callback(__rte_unused void *ptr_params,
> __rte_unused struct cmdline *ctx,
Please don't do unrelated whitespace changes.
>
> -
> static void
> -pcmd_rxmode_callback(void *ptr_params,
> - __rte_unused struct cmdline *ctx,
> - __rte_unused void *ptr_data)
> +pcmd_rxmode_callback(void *parsed_result,
> + __rte_unused struct cmdline *cl,
> + __rte_unused void *data)
> {
> - struct pcmd_intstr_params *params = ptr_params;
> - int stat;
> + int ret = -ENOTSUP;
> + uint16_t vf_rxmode = 0;
> + struct cmd_set_vf_rxmode *res = parsed_result;
> +
> + int is_on = (strcmp(res->on, "on") == 0) ? 1 : 0;
Simpler as:
bool is_on = !strcmp(res->on, "on");
Also what if use gives bogus value for on/off?
> + if (!strcmp(res->what, "rxmode")) {
> + if (!strcmp(res->mode, "AUPE"))
> + vf_rxmode |= RTE_ETH_VMDQ_ACCEPT_UNTAG;
> + else if (!strcmp(res->mode, "ROPE"))
> + vf_rxmode |= RTE_ETH_VMDQ_ACCEPT_HASH_UC;
> + else if (!strcmp(res->mode, "BAM"))
> + vf_rxmode |= RTE_ETH_VMDQ_ACCEPT_BROADCAST;
> + else if (!strncmp(res->mode, "MPE", 3))
> + vf_rxmode |= RTE_ETH_VMDQ_ACCEPT_MULTICAST;
> + }
You need to handle the "none of the above" case.
>
> - if (!rte_eth_dev_is_valid_port(params->port)) {
> - printf("Error: Invalid port number %i\n", params->port);
> - return;
> + RTE_SET_USED(is_on);
> + RTE_SET_USED(vf_rxmode);
> +
> +#ifdef RTE_NET_IXGBE
> + if (ret == -ENOTSUP) {
> + ret = rte_pmd_ixgbe_set_vf_rxmode(res->port_id, res->vf_id,
> + vf_rxmode, (uint8_t)is_on);
> + if (ret == -ENOTSUP)
> + printf("ixgbe not supported\n");
> }
> - stat = rte_ethtool_net_set_rx_mode(params->port);
> - if (stat == 0)
> - return;
> - else if (stat == -ENOTSUP)
> - printf("Port %i: Operation not supported\n", params->port);
> - else
> - printf("Port %i: Error setting rx mode\n", params->port);
> +#endif
> +#ifdef RTE_NET_BNXT
> + if (ret == -ENOTSUP) {
> + ret = rte_pmd_bnxt_set_vf_rxmode(res->port_id, res->vf_id,
> + vf_rxmode, (uint8_t)is_on);
> + if (ret == -ENOTSUP)
> + printf("bnxt not supported\n");
> + }
> +#endif
This is a mess. And it shows missing ethdev functionality.
Calling a driver with some unrelated device may break it.
Not sure if anyone really cares about ethtool?
But if they do please fixup and resubmit
next prev parent reply other threads:[~2023-07-03 22:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-25 6:12 [PATCH] " Muhammad Jawad Hussain
2022-08-11 12:58 ` [PATCH v2] " Muhammad Jawad Hussain
2023-07-03 22:19 ` Stephen Hemminger [this message]
2022-05-29 14:28 Muhammad Jawad Hussain
2022-07-06 7:16 ` Muhammad Jawad Hussain
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=20230703151944.62af33ab@hermes.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=jawad.hussain@emumba.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 NNTP newsgroup(s).