Re: [dpdk-dev] [PATCH v2 08/10] net/sfc: support action VXLAN ENCAP in MAE backend

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 4/1/21 2:36 AM, Ivan Malov wrote:
> Hi,
>
> On 01/04/2021 02:21, Thomas Monjalon wrote:
>> 12/03/2021 12:07, Ivan Malov:
>>> +static int
>>> +sfc_mae_encap_header_add(struct sfc_adapter *sa,
>>> +             const struct sfc_mae_bounce_eh *bounce_eh,
>>> +             struct sfc_mae_encap_header **encap_headerp)
>>> +{
>>> +    struct sfc_mae_encap_header *encap_header;
>>> +    struct sfc_mae *mae = &sa->mae;
>>> +
>>> +    SFC_ASSERT(sfc_adapter_is_locked(sa));
>>> +
>>> +    encap_header = rte_zmalloc("sfc_mae_encap_header",
>>> +                   sizeof(*encap_header), 0);
>>> +    if (encap_header == NULL)
>>> +        return ENOMEM;
>>> +
>>> +    encap_header->size = bounce_eh->size;
>>> +
>>> +    encap_header->buf = rte_malloc("sfc_mae_encap_header_buf",
>>> +                       encap_header->size, 0);
>>> +    if (encap_header->buf == NULL) {
>>> +        rte_free(encap_header);
>>> +        return ENOMEM;
>>> +    }
>>
>> Are the error codes positives on purpose?
>> checkpatch is throwing this warning:
>> USE_NEGATIVE_ERRNO: return of an errno should typically be negative
>> (ie: return -ENOMEM)
>
> Kind of yes, on purpose. It has been like that for a long time
> already; it's simpler to keep errors positive in all such small
> internal helpers and then negate the result in the place where
> rte_flow_error_set() is used. We understand the concern of yours; our
> code is tested for error path correctness every now and again. If
> there're some inconsistencies, we are ready to fix such in no time.

Yes, unfortunately base driver uses positive error codes.

The idea here is that interface between DPDK PMD and base
driver is much wider than interface between DPDK PMD and
ethdev. That's why it is much easier to care about converting
from positive to negative on PMD-ethdev border. Also rte_errno
is positive.

>> Also the base code has a lot of these warnings:
>> RETURN_PARENTHESES: return is not a function, parentheses are not
>> required

Base driver code was initially created for FreeBSD and Solaris which require
parenthesis around return value.

>> I guess you cannot do anything to avoid it in base code?
>
> Yes, your understanding is correct. Sorry for the inconvenience.
>