Hi Akhil,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Thursday, May 18, 2023 2:16 PM
> To: Anoob Joseph <ano...@marvell.com>; Thomas Monjalon
> <tho...@monjalon.net>; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
> Konstantin Ananyev <konstantin.v.anan...@yandex.ru>; Bernard
> Iremonger <bernard.iremon...@intel.com>
> Cc: Hemant Agrawal <hemant.agra...@nxp.com>; Mattias Rönnblom
> <mattias.ronnb...@ericsson.com>; Kiran Kumar Kokkilagadda
> <kirankum...@marvell.com>; Volodymyr Fialko <vfia...@marvell.com>;
> dev@dpdk.org; Olivier Matz <olivier.m...@6wind.com>
> Subject: RE: [PATCH v2 02/22] lib: add pdcp protocol
>
> > > > > > diff --git a/lib/pdcp/rte_pdcp.c b/lib/pdcp/rte_pdcp.c new
> > > > > > file mode
> > > > > > 100644 index 0000000000..8914548dbd
> > > > > > --- /dev/null
> > > > > > +++ b/lib/pdcp/rte_pdcp.c
> > > > > > @@ -0,0 +1,138 @@
> > > > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > > > + * Copyright(C) 2023 Marvell.
> > > > > > + */
> > > > > > +
> > > > > > +#include <rte_errno.h>
> > > > > > +#include <rte_pdcp.h>
> > > > > > +#include <rte_malloc.h>
> > > > > > +
> > > > > > +#include "pdcp_crypto.h"
> > > > > > +#include "pdcp_entity.h"
> > > > > > +#include "pdcp_process.h"
> > > > > > +
> > > > > > +static int
> > > > > > +pdcp_entity_size_get(const struct rte_pdcp_entity_conf *conf) {
> > > > > > + int size;
> > > > > > +
> > > > > > + size = sizeof(struct rte_pdcp_entity) + sizeof(struct
> > > > > > +entity_priv);
> > > > > > +
> > > > > > + if (conf->pdcp_xfrm.pkt_dir ==
> > RTE_SECURITY_PDCP_DOWNLINK)
> > > > > > + size += sizeof(struct entity_priv_dl_part);
> > > > > > + else if (conf->pdcp_xfrm.pkt_dir ==
> > RTE_SECURITY_PDCP_UPLINK)
> > > > > > + size += sizeof(struct entity_priv_ul_part);
> > > > > > + else
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + return RTE_ALIGN_CEIL(size, RTE_CACHE_LINE_SIZE); }
> > > > > > +
> > > > > > +struct rte_pdcp_entity *
> > > > > > +rte_pdcp_entity_establish(const struct rte_pdcp_entity_conf
> *conf) {
> > > > > > + struct rte_pdcp_entity *entity = NULL;
> > > > > > + struct entity_priv *en_priv;
> > > > > > + int ret, entity_size;
> > > > > > +
> > > > > > + if (conf == NULL || conf->cop_pool == NULL) {
> > > > > > + rte_errno = -EINVAL;
> > > > > > + return NULL;
> > > > > > + }
> > > > >
> > > > > errnos are normally set as positive values.
> > > >
> > > > [Anoob] I do not think so. I checked rte_ethdev.h, rte_flow.h etc
> > > > and all APIs are returning negative values in case of errors.
> > >
> > > Check again lib/ethdev/rte_ethdev.c
> > > rte_errno are set as positive values only and APIs return error
> > > numbers as negative values.
> >
> > [Anoob] Okay. There are many APIs were this is not done correctly
> > (check
> > rte_flow.h) . For lib PDCP additions, I'll have this handled. Some of
> > these conventions should be documented to avoid confusion.
>
> I am not sure what you are referring to.
> I cannot find any discrepancy in rte_flow.c as well.
>
> Can you give an example where it is wrong. We can ask specific people to fix
> that.
>
> Regarding documentation.
> It is mentioned in rte_errno.h
> /**
> * Error number value, stored per-thread, which can be queried after
> * calls to certain functions to determine why those functions failed.
> *
> * Uses standard values from errno.h wherever possible, with a small
> number
> * of additional possible values for RTE-specific conditions.
> */
> #define rte_errno RTE_PER_LCORE(_rte_errno)
>
> And errno.h has all positive values defined.
[Anoob] Agreed. There are descriptions in rte_flow.h which got us confused.
Thanks for the clarification. Changes in PDCP will come in next version.
