Agree, it's confusing if internal functions have odp_ prefix. You could name 
those packet_hdr_has_l2(), etc which is intuitive since input is 
odp_packet_hdr_t. odp_packet_internal.h has also other packet_ or packet_hdr_ 
prefixed functions.

-Petri 


> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of EXT
> Maxim Uvarov
> Sent: Tuesday, March 15, 2016 5:14 PM
> To: [email protected]
> Subject: Re: [lng-odp] [PATCH 1/2] linux-generic: packet_flags: use
> accessors to modify eth and l2 flag
> 
> Zoltan,
> 
> you can not name internal functions with odp_ prefix.
> 
> So 2 options or move that to API:
> odp_packet_hdr_has_l2
> odp_packet_hdr_has_eth
> 
> or rename to
> 
> _odp_....
> 
> Best regards,
> Maxim.
> 
> 
> On 03/14/16 09:51, Bala Manoharan wrote:
> > Reviewed-by: Balasubramanian Manoharan <[email protected]
> > <mailto:[email protected]>>
> >
> Please be careful with review.
> 
> > Regards,
> > Bala
> >
> > On 11 March 2016 at 14:03, Zoltan Kiss <[email protected]
> > <mailto:[email protected]>> wrote:
> >
> >     Ping
> >
> >     On 03/03/16 03:05, Zoltan Kiss wrote:
> >
> >         This makes it possible for other implementations like ODP-DPDK
> >         to reuse
> >         classification code while using a different packet API.
> >
> >         Signed-off-by: Zoltan Kiss <[email protected]
> >         <mailto:[email protected]>>
> >         ---
> >         platform/linux-generic/include/odp_classification_inlines.h |
> >         2 +-
> >         platform/linux-generic/include/odp_packet_internal.h       |
> >         10 ++++++++++
> >           platform/linux-generic/odp_classification.c        |  4 ++--
> >           3 files changed, 13 insertions(+), 3 deletions(-)
> >
> >         diff --git
> >         a/platform/linux-generic/include/odp_classification_inlines.h
> >         b/platform/linux-generic/include/odp_classification_inlines.h
> >         index 96cf77e..2318349 100644
> >         --- a/platform/linux-
> generic/include/odp_classification_inlines.h
> >         +++ b/platform/linux-
> generic/include/odp_classification_inlines.h
> >         @@ -162,7 +162,7 @@ static inline int verify_pmr_dmac(const
> >         uint8_t *pkt_addr,
> >                 uint64_t dmac_be = 0;
> >                 const odph_ethhdr_t *eth;
> >           -     if (!pkt_hdr->input_flags.eth)
> >         +       if (!odp_packet_hdr_has_eth(pkt_hdr))
> >                         return 0;
> >                 eth = (const odph_ethhdr_t *)(pkt_addr +
> >         pkt_hdr->l2_offset);
> >         diff --git
> >         a/platform/linux-generic/include/odp_packet_internal.h
> >         b/platform/linux-generic/include/odp_packet_internal.h
> >         index 85d4924..d9fe544 100644
> >         --- a/platform/linux-generic/include/odp_packet_internal.h
> >         +++ b/platform/linux-generic/include/odp_packet_internal.h
> >         @@ -258,6 +258,16 @@ odp_buffer_t
> >         _odp_packet_to_buffer(odp_packet_t pkt);
> >           /* Convert a buffer handle to a packet handle */
> >           odp_packet_t _odp_packet_from_buffer(odp_buffer_t buf);
> >           +static inline int odp_packet_hdr_has_l2(odp_packet_hdr_t
> >         *pkt_hdr)
> >         +{
> >         +       return pkt_hdr->input_flags.l2;
> >         +}
> >         +
> >         +static inline int odp_packet_hdr_has_eth(odp_packet_hdr_t
> >         *pkt_hdr)
> >         +{
> >         +       return pkt_hdr->input_flags.eth;
> >         +}
> >         +
> >           int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const
> >         uint8_t *parseptr);
> >             int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const
> >         uint8_t *parseptr);
> >         diff --git a/platform/linux-generic/odp_classification.c
> >         b/platform/linux-generic/odp_classification.c
> >         index da195ad..4551951 100644
> >         --- a/platform/linux-generic/odp_classification.c
> >         +++ b/platform/linux-generic/odp_classification.c
> >         @@ -1003,8 +1003,8 @@ cos_t *match_qos_l2_cos(pmr_l2_cos_t
> >         *l2_cos, const uint8_t *pkt_addr,
> >                 const odph_vlanhdr_t *vlan;
> >                 uint16_t qos;
> >           -     if (hdr->input_flags.l2 && hdr->input_flags.vlan &&
> >         -           hdr->input_flags.eth) {
> >         +       if (odp_packet_hdr_has_l2(hdr) && hdr->input_flags.vlan
> &&
> >         +           odp_packet_hdr_has_eth(hdr)) {
> >                         eth = (const odph_ethhdr_t *)(pkt_addr +
> >         hdr->l2_offset);
> >                         vlan = (const odph_vlanhdr_t *)(&eth->type);
> >                         qos = odp_be_to_cpu_16(vlan->tci);
> >
> >
> >
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > [email protected]
> > https://lists.linaro.org/mailman/listinfo/lng-odp
> 
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to