>-----Original Message-----
>From: Ilya Maximets <i.maxim...@samsung.com>
>Sent: 10 September 2019 14:29
>To: Konieczny, TomaszX <tomaszx.koniec...@intel.com>; d...@openvswitch.org
>Cc: Stokes, Ian <ian.sto...@intel.com>
>Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.
>
>
>I looked at the code once more and to the dpdk drivers' implementation.
>You're partially right, but we have to get() flow control configuration in all
>cases
>because some drivers (e.g. ixgbe) has flow control enabled by default and since
>we have 'false' as a default value, we need to disable the flow control for
>it. Not
>calling the get() will result in setting zeroized fc_config, which may be
>discarded
>by the driver since those values are validated inside.
>
>So, I think that we need to call get() function unconditionally.
>Avoiding of failure for devices that doesn't support flow control could be
>implemented like this:
>
> /* Get the Flow control configuration. */
> err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
> if (err) {
> if (err == -ENOTSUP) {
> VLOG_INFO_ONCE("%s: Flow control is not supported.",
> netdev_get_name(netdev));
> err = 0; /* Not fatal. */
> } else {
> VLOG_WARN("%s: Cannot get flow control parameters: %s",
> netdev_get_name(netdev), rte_strerror(-err));
> }
> goto out;
> }
>
>What do you think?
>
>
>One minor nit is that it's better to keep an empty line here:
>---
> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false);
>-
> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
>---
>
>Best regards, Ilya Maximets.
Hi Ilya,
I agree, this approach looks neater and seems to work.
However now if you try to enable flow control on unsupported device it will
pass without any warnings or errors. Would it be a desired behavior?
Also, I've tested my patch on ixgbe device and it worked fine, though I still
like your solution better.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
