>-----Original Message-----
>From: Ilya Maximets <i.maxim...@samsung.com>
>Sent: 11 September 2019 12:26
>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.
>
>On 11.09.2019 13:16, Konieczny, TomaszX wrote:
>>
>>
>>> -----Original Message-----
>>> From: Ilya Maximets <i.maxim...@samsung.com>
>>> Sent: 11 September 2019 12:08
>>> 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.
>>>
>>> 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;
>> }
>>
>
>As I said, we can't use VLOG_INFO_ONCE, because the message will be printed
>only once in a process lifetime, i.e. only for the first netdev that will
>reach this
>code. For all later added netdevs the message will not be printed.
So we might as well omit any VLOG if flow control not supported and not
requested.
if (err == -ENOTSUP && fc_mode == RTE_FC_NONE && autoneg == false) {
err = 0; /* Not fatal. Not requested. */
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
