On Thu, 11 Aug 2022 17:58:40 +0500
Muhammad Jawad Hussain <[email protected]> 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
