[ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.

[Konieczny, TomaszX]

>-----Original Message-----
>From: Ilya Maximets <i.maximets at samsung.com>
>Sent: 11 September 2019 12:08
>To: Konieczny, TomaszX <tomaszx.konieczny at intel.com>; dev at openvswitch.org
>Cc: Stokes, Ian <ian.stokes at intel.com>
>Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.
>
>On 11.09.2019 12:53, Konieczny, TomaszX wrote:
>>
>> 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?
>
>We could change VLOG_INFO_ONCE to VLOG_WARN, in this case where will be
>one warning in the log for each configuration attempt.
>Basically, *_ONCE logging is not suitable here, because it will warn only on the
>first device. My bad.
>
>Ideally, we need a separate configuration knob to control if OVS needs to touch
>flow control at all. And print a warning only if user enabled this feature. I'm not
>sure how user-friendly is this.
>For now, WARN on each configuration attempt might be fine.
>So, s/VLOG_INFO_ONCE/VLOG_WARN/ .
>
>What do you think?
>Ian, maybe you have something to add?
>
>>
>> Also, I've tested my patch on ixgbe device and it worked fine, though I still like
>your solution better.

I was thinking of something like this:

if (err) {
        if (err == -ENOTSUP && fc_mode == RTE_FC_NONE && autoneg == false) {
            VLOG_INFO_ONCE("%s: Flow control is not supported.",
                           netdev_get_name(netdev));
            err = 0; /* Not fatal. */
        } else if (err == -ENOTSUP) {
            VLOG_WARN("%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;
    }