| Message ID | 1453307831-15459-2-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
I was directly accessing this value to avoid a function call.
If required you can use the existing ODP API odp_packet_len() which
returns the total length.
Maybe you can move this as an inline function.
code snippet from odp_packet.c
=======
uint32_t odp_packet_len(odp_packet_t pkt)
{
return odp_packet_hdr(pkt)->frame_len;
}
Regards,
Bala
Regards,
Bala
On 21 January 2016 at 13:51, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>
>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>> Zoltan Kiss
>> Sent: Wednesday, January 20, 2016 6:37 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH 1/2] linux-generic: packet: hide frame_len
>> behind accessor
>>
>> The classification code accesses this variable directly, which prevents
>> reusing that code e.g. in ODP-DPDK.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>> platform/linux-generic/include/odp_classification_inlines.h | 5 +++--
>> platform/linux-generic/include/odp_packet_internal.h | 5 +++++
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp_classification_inlines.h
>> b/platform/linux-generic/include/odp_classification_inlines.h
>> index e9739aa..e4d87c9 100644
>> --- a/platform/linux-generic/include/odp_classification_inlines.h
>> +++ b/platform/linux-generic/include/odp_classification_inlines.h
>> @@ -24,6 +24,7 @@ extern "C" {
>> #include <odp/helper/ipsec.h>
>> #include <odp/helper/udp.h>
>> #include <odp/helper/tcp.h>
>> +#include <odp_packet_internal.h>
>>
>> /* PMR term value verification function
>> These functions verify the given PMR term value with the value in the
>> packet
>> @@ -33,7 +34,7 @@ These following functions return 1 on success and 0 on
>> failure
>> static inline int verify_pmr_packet_len(odp_packet_hdr_t *pkt_hdr,
>> pmr_term_value_t *term_value)
>> {
>> - if (term_value->val == (pkt_hdr->frame_len &
>> + if (term_value->val == (packet_hdr_len(pkt_hdr) &
>> term_value->mask))
>> return 1;
>>
>> @@ -240,7 +241,7 @@ static inline int verify_pmr_custom_frame(const
>> uint8_t *pkt_addr,
>>
>> ODP_ASSERT(val_sz <= ODP_PMR_TERM_BYTES_MAX);
>>
>> - if (pkt_hdr->frame_len <= offset + val_sz)
>> + if (packet_hdr_len(pkt_hdr) <= offset + val_sz)
>> return 0;
>>
>> memcpy(&val, pkt_addr + offset, val_sz);
>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
>> b/platform/linux-generic/include/odp_packet_internal.h
>> index 12e9cca..0b2e9d0 100644
>> --- a/platform/linux-generic/include/odp_packet_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_internal.h
>> @@ -214,6 +214,11 @@ static inline void pull_tail(odp_packet_hdr_t
>> *pkt_hdr, size_t len)
>> pkt_hdr->frame_len -= len;
>> }
>>
>> +static inline uint32_t packet_hdr_len(odp_packet_hdr_t* pkt_hdr)
>
> Since it's an accessor for frame length and not header length, better call it packet_frame_len() or packet_len() (which matches packet_set_len() under).
>
> -Petri
>
>
>> +{
>> + return pkt_hdr->frame_len;
>> +}
>> +
>> static inline void packet_set_len(odp_packet_t pkt, uint32_t len)
>> {
>> odp_packet_hdr(pkt)->frame_len = len;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
On 21/01/16 08:49, Bala Manoharan wrote: > I was directly accessing this value to avoid a function call. > If required you can use the existing ODP API odp_packet_len() which > returns the total length. > Maybe you can move this as an inline function. > > code snippet from odp_packet.c > ======= > uint32_t odp_packet_len(odp_packet_t pkt) The problem with odp_packet_len is that it works on odp_packet_t, not odp_packet_hdr_t. On ODP-DPDK they are the same, but not in linux-generic. Even if I would make it inline, and use odp_packet_len(odp_hdr_to_buf(pkt_hdr)), it would involve an another odp_buf_to_hdr conversion, which is not that cheap. It's easier to use an internal function which accesses the length from a header pointer. And in my patch it's already inline, so there is no hurt on performance. Zoli > { > return odp_packet_hdr(pkt)->frame_len; > } > > Regards, > Bala > Regards, > Bala > > > On 21 January 2016 at 13:51, Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com> wrote: >> >> >>> -----Original Message----- >>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT >>> Zoltan Kiss >>> Sent: Wednesday, January 20, 2016 6:37 PM >>> To: lng-odp@lists.linaro.org >>> Subject: [lng-odp] [PATCH 1/2] linux-generic: packet: hide frame_len >>> behind accessor >>> >>> The classification code accesses this variable directly, which prevents >>> reusing that code e.g. in ODP-DPDK. >>> >>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >>> --- >>> platform/linux-generic/include/odp_classification_inlines.h | 5 +++-- >>> platform/linux-generic/include/odp_packet_internal.h | 5 +++++ >>> 2 files changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/odp_classification_inlines.h >>> b/platform/linux-generic/include/odp_classification_inlines.h >>> index e9739aa..e4d87c9 100644 >>> --- a/platform/linux-generic/include/odp_classification_inlines.h >>> +++ b/platform/linux-generic/include/odp_classification_inlines.h >>> @@ -24,6 +24,7 @@ extern "C" { >>> #include <odp/helper/ipsec.h> >>> #include <odp/helper/udp.h> >>> #include <odp/helper/tcp.h> >>> +#include <odp_packet_internal.h> >>> >>> /* PMR term value verification function >>> These functions verify the given PMR term value with the value in the >>> packet >>> @@ -33,7 +34,7 @@ These following functions return 1 on success and 0 on >>> failure >>> static inline int verify_pmr_packet_len(odp_packet_hdr_t *pkt_hdr, >>> pmr_term_value_t *term_value) >>> { >>> - if (term_value->val == (pkt_hdr->frame_len & >>> + if (term_value->val == (packet_hdr_len(pkt_hdr) & >>> term_value->mask)) >>> return 1; >>> >>> @@ -240,7 +241,7 @@ static inline int verify_pmr_custom_frame(const >>> uint8_t *pkt_addr, >>> >>> ODP_ASSERT(val_sz <= ODP_PMR_TERM_BYTES_MAX); >>> >>> - if (pkt_hdr->frame_len <= offset + val_sz) >>> + if (packet_hdr_len(pkt_hdr) <= offset + val_sz) >>> return 0; >>> >>> memcpy(&val, pkt_addr + offset, val_sz); >>> diff --git a/platform/linux-generic/include/odp_packet_internal.h >>> b/platform/linux-generic/include/odp_packet_internal.h >>> index 12e9cca..0b2e9d0 100644 >>> --- a/platform/linux-generic/include/odp_packet_internal.h >>> +++ b/platform/linux-generic/include/odp_packet_internal.h >>> @@ -214,6 +214,11 @@ static inline void pull_tail(odp_packet_hdr_t >>> *pkt_hdr, size_t len) >>> pkt_hdr->frame_len -= len; >>> } >>> >>> +static inline uint32_t packet_hdr_len(odp_packet_hdr_t* pkt_hdr) >> >> Since it's an accessor for frame length and not header length, better call it packet_frame_len() or packet_len() (which matches packet_set_len() under). >> >> -Petri >> >> >>> +{ >>> + return pkt_hdr->frame_len; >>> +} >>> + >>> static inline void packet_set_len(odp_packet_t pkt, uint32_t len) >>> { >>> odp_packet_hdr(pkt)->frame_len = len; >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
On 21/01/16 08:21, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT >> Zoltan Kiss >> Sent: Wednesday, January 20, 2016 6:37 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH 1/2] linux-generic: packet: hide frame_len >> behind accessor >> >> The classification code accesses this variable directly, which prevents >> reusing that code e.g. in ODP-DPDK. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >> --- >> platform/linux-generic/include/odp_classification_inlines.h | 5 +++-- >> platform/linux-generic/include/odp_packet_internal.h | 5 +++++ >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp_classification_inlines.h >> b/platform/linux-generic/include/odp_classification_inlines.h >> index e9739aa..e4d87c9 100644 >> --- a/platform/linux-generic/include/odp_classification_inlines.h >> +++ b/platform/linux-generic/include/odp_classification_inlines.h >> @@ -24,6 +24,7 @@ extern "C" { >> #include <odp/helper/ipsec.h> >> #include <odp/helper/udp.h> >> #include <odp/helper/tcp.h> >> +#include <odp_packet_internal.h> >> >> /* PMR term value verification function >> These functions verify the given PMR term value with the value in the >> packet >> @@ -33,7 +34,7 @@ These following functions return 1 on success and 0 on >> failure >> static inline int verify_pmr_packet_len(odp_packet_hdr_t *pkt_hdr, >> pmr_term_value_t *term_value) >> { >> - if (term_value->val == (pkt_hdr->frame_len & >> + if (term_value->val == (packet_hdr_len(pkt_hdr) & >> term_value->mask)) >> return 1; >> >> @@ -240,7 +241,7 @@ static inline int verify_pmr_custom_frame(const >> uint8_t *pkt_addr, >> >> ODP_ASSERT(val_sz <= ODP_PMR_TERM_BYTES_MAX); >> >> - if (pkt_hdr->frame_len <= offset + val_sz) >> + if (packet_hdr_len(pkt_hdr) <= offset + val_sz) >> return 0; >> >> memcpy(&val, pkt_addr + offset, val_sz); >> diff --git a/platform/linux-generic/include/odp_packet_internal.h >> b/platform/linux-generic/include/odp_packet_internal.h >> index 12e9cca..0b2e9d0 100644 >> --- a/platform/linux-generic/include/odp_packet_internal.h >> +++ b/platform/linux-generic/include/odp_packet_internal.h >> @@ -214,6 +214,11 @@ static inline void pull_tail(odp_packet_hdr_t >> *pkt_hdr, size_t len) >> pkt_hdr->frame_len -= len; >> } >> >> +static inline uint32_t packet_hdr_len(odp_packet_hdr_t* pkt_hdr) > > Since it's an accessor for frame length and not header length, better call it packet_frame_len() or packet_len() (which matches packet_set_len() under). Makes sense, I'll use packet_len() > > -Petri > > >> +{ >> + return pkt_hdr->frame_len; >> +} >> + >> static inline void packet_set_len(odp_packet_t pkt, uint32_t len) >> { >> odp_packet_hdr(pkt)->frame_len = len; >> -- >> 1.9.1 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/odp_classification_inlines.h b/platform/linux-generic/include/odp_classification_inlines.h index e9739aa..e4d87c9 100644 --- a/platform/linux-generic/include/odp_classification_inlines.h +++ b/platform/linux-generic/include/odp_classification_inlines.h @@ -24,6 +24,7 @@ extern "C" { #include <odp/helper/ipsec.h> #include <odp/helper/udp.h> #include <odp/helper/tcp.h> +#include <odp_packet_internal.h> /* PMR term value verification function These functions verify the given PMR term value with the value in the packet @@ -33,7 +34,7 @@ These following functions return 1 on success and 0 on failure static inline int verify_pmr_packet_len(odp_packet_hdr_t *pkt_hdr, pmr_term_value_t *term_value) { - if (term_value->val == (pkt_hdr->frame_len & + if (term_value->val == (packet_hdr_len(pkt_hdr) & term_value->mask)) return 1; @@ -240,7 +241,7 @@ static inline int verify_pmr_custom_frame(const uint8_t *pkt_addr, ODP_ASSERT(val_sz <= ODP_PMR_TERM_BYTES_MAX); - if (pkt_hdr->frame_len <= offset + val_sz) + if (packet_hdr_len(pkt_hdr) <= offset + val_sz) return 0; memcpy(&val, pkt_addr + offset, val_sz); diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h index 12e9cca..0b2e9d0 100644 --- a/platform/linux-generic/include/odp_packet_internal.h +++ b/platform/linux-generic/include/odp_packet_internal.h @@ -214,6 +214,11 @@ static inline void pull_tail(odp_packet_hdr_t *pkt_hdr, size_t len) pkt_hdr->frame_len -= len; } +static inline uint32_t packet_hdr_len(odp_packet_hdr_t* pkt_hdr) +{ + return pkt_hdr->frame_len; +} + static inline void packet_set_len(odp_packet_t pkt, uint32_t len) { odp_packet_hdr(pkt)->frame_len = len;
The classification code accesses this variable directly, which prevents reusing that code e.g. in ODP-DPDK. Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> --- platform/linux-generic/include/odp_classification_inlines.h | 5 +++-- platform/linux-generic/include/odp_packet_internal.h | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-)