On 26 March 2015 at 15:02, Savolainen, Petri (Nokia - FI/Espoo) < [email protected]> wrote:
> I was referring to flags in a SoC packet descriptor, which may be stored > into one or more u64 words. > Code could access the most significant half (32 bits) of such 64-bit words using 32-bit loads so avoid any need to shift. Anyway I don't think a shift operation will cause any measurable overhead, it might be executed in parallel with other instructions. > I’m also suggesting to use int. Just wanted check if SoC vendors agree > that it performs OK. > > > > -Petri > > > > > > > > *From:* ext Bill Fischofer [mailto:[email protected]] > *Sent:* Thursday, March 26, 2015 3:13 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* ext Maxim Uvarov; [email protected] > *Subject:* Re: [lng-odp] [API-NEXT PATCHv2 1/3] api: packet: use > odp_bool_t for packet flag APIs > > > > We actually have far fewer than 32 flag bits defined (19 at present), so > no need for uint64_t to store them. Most SoCs have fairly efficient > bit-twiddling instructions anyway so I'd leave the optimal implementation > questions to them. > > > > Sounds like we should just leave this as int (as it is today), and just > update the doxygen and the test to check for non-zero instead of 1. > > > > > > On Thu, Mar 26, 2015 at 7:55 AM, Savolainen, Petri (Nokia - FI/Espoo) < > [email protected]> wrote: > > It’s OK to me. Although, I was thinking that an implementation defined > type can save e.g. cycle in a case like this > > > > - flags are stored in a uint64_t > > - IPv4 flag in upper 32 bits > > - implementation defines the type as uint64_t > > - only one mask operation is needed e.g. > > return pkt_hrd.flags & HAS_IPV4_MASK; > > > > vs > > > > - return type is int > > - a mask and a shift are needed e.g. > > return (int)((pkt_hrd.flags & HAS_IPV4_MASK) >> 32); > > > > Is this a significant penalty? Only flags in the upper 32 bits need to be > shifted. > > > > > > -Petri > > > > > > *From:* ext Bill Fischofer [mailto:[email protected]] > *Sent:* Thursday, March 26, 2015 2:13 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* ext Maxim Uvarov; [email protected] > > > *Subject:* Re: [lng-odp] [API-NEXT PATCHv2 1/3] api: packet: use > odp_bool_t for packet flag APIs > > > > Is there any reason not to leave this as int then? > > > > On Thu, Mar 26, 2015 at 7:10 AM, Savolainen, Petri (Nokia - FI/Espoo) < > [email protected]> wrote: > > As discussed in one of the calls, I think 0 vs. non-zero return value is > OK, but odp_bool_t would be misleading type for that range (bool is 0 or > 1). So, it would be better to have another type for true/false integer > condition. > > -Petri > > > > -----Original Message----- > > From: ext Maxim Uvarov [mailto:[email protected]] > > Sent: Thursday, March 26, 2015 11:39 AM > > To: [email protected] > > Cc: Petri Savolainen > > Subject: Re: [lng-odp] [API-NEXT PATCHv2 1/3] api: packet: use odp_bool_t > > for packet flag APIs > > > > Petri, please review. > > > > Maxim. > > > > On 03/20/15 21:14, Bill Fischofer wrote: > > > This change addresses Bug https://bugs.linaro.org/show_bug.cgi?id=1334 > > > > > > Signed-off-by: Bill Fischofer <[email protected]> > > > --- > > > > > > v2 adds doxygen fix to this patch. Petri: Please review this for API > > > conformance. > > > > > > include/odp/api/packet_flags.h | 72 > +++++++++++++++++++++------------- > > -------- > > > 1 file changed, 36 insertions(+), 36 deletions(-) > > > > > > diff --git a/include/odp/api/packet_flags.h > > b/include/odp/api/packet_flags.h > > > index b1e179e..ccfb747 100644 > > > --- a/include/odp/api/packet_flags.h > > > +++ b/include/odp/api/packet_flags.h > > > @@ -32,163 +32,163 @@ extern "C" { > > > * Checks all error flags at once. > > > * > > > * @param pkt Packet handle > > > - * @retval 1 packet has errors > > > + * @retval non-zero packet has errors > > > * @retval 0 packet has no errors > > > */ > > > -int odp_packet_has_error(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_error(odp_packet_t pkt); > > > > > > /** > > > * Check for L2 header, e.g. ethernet > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a valid & known L2 header > > > + * @retval non-zero if packet contains a valid & known L2 header > > > * @retval 0 if packet does not contain a valid & known L2 header > > > */ > > > -int odp_packet_has_l2(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_l2(odp_packet_t pkt); > > > > > > /** > > > * Check for L3 header, e.g. IPv4, IPv6 > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a valid & known L3 header > > > + * @retval non-zero if packet contains a valid & known L3 header > > > * @retval 0 if packet does not contain a valid & known L3 header > > > */ > > > -int odp_packet_has_l3(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_l3(odp_packet_t pkt); > > > > > > /** > > > * Check for L4 header, e.g. UDP, TCP, SCTP (also ICMP) > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a valid & known L4 header > > > + * @retval non-zero if packet contains a valid & known L4 header > > > * @retval 0 if packet does not contain a valid & known L4 header > > > */ > > > -int odp_packet_has_l4(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_l4(odp_packet_t pkt); > > > > > > /** > > > * Check for Ethernet header > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a valid eth header > > > + * @retval non-zero if packet contains a valid eth header > > > * @retval 0 if packet does not contain a valid & known eth header > > > */ > > > -int odp_packet_has_eth(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_eth(odp_packet_t pkt); > > > > > > /** > > > * Check for jumbo frame > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a jumbo frame > > > + * @retval non-zero if packet contains a jumbo frame > > > * @retval 0 if packet does not contain a jumbo frame > > > */ > > > -int odp_packet_has_jumbo(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_jumbo(odp_packet_t pkt); > > > > > > /** > > > * Check for VLAN > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a VLAN header > > > + * @retval non-zero if packet contains a VLAN header > > > * @retval 0 if packet does not contain a VLAN header > > > */ > > > -int odp_packet_has_vlan(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_vlan(odp_packet_t pkt); > > > > > > /** > > > * Check for VLAN QinQ (stacked VLAN) > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a VLAN QinQ header > > > + * @retval non-zero if packet contains a VLAN QinQ header > > > * @retval 0 if packet does not contain a VLAN QinQ header > > > */ > > > -int odp_packet_has_vlan_qinq(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_vlan_qinq(odp_packet_t pkt); > > > > > > /** > > > * Check for ARP > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains an ARP message > > > + * @retval non-zero if packet contains an ARP message > > > * @retval 0 if packet does not contain an ARP message > > > */ > > > -int odp_packet_has_arp(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_arp(odp_packet_t pkt); > > > > > > /** > > > * Check for IPv4 > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains an IPv4 header > > > + * @retval non-zero if packet contains an IPv4 header > > > * @retval 0 if packet does not contain an IPv4 header > > > */ > > > -int odp_packet_has_ipv4(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_ipv4(odp_packet_t pkt); > > > > > > /** > > > * Check for IPv6 > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains an IPv6 header > > > + * @retval non-zero if packet contains an IPv6 header > > > * @retval 0 if packet does not contain an IPv6 header > > > */ > > > -int odp_packet_has_ipv6(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_ipv6(odp_packet_t pkt); > > > > > > /** > > > * Check for IP fragment > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet is an IP fragment > > > + * @retval non-zero if packet is an IP fragment > > > * @retval 0 if packet is not an IP fragment > > > */ > > > -int odp_packet_has_ipfrag(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_ipfrag(odp_packet_t pkt); > > > > > > /** > > > * Check for IP options > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains IP options > > > + * @retval non-zero if packet contains IP options > > > * @retval 0 if packet does not contain IP options > > > */ > > > -int odp_packet_has_ipopt(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_ipopt(odp_packet_t pkt); > > > > > > /** > > > * Check for IPSec > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet requires IPSec processing > > > + * @retval non-zero if packet requires IPSec processing > > > * @retval 0 if packet does not require IPSec processing > > > */ > > > -int odp_packet_has_ipsec(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_ipsec(odp_packet_t pkt); > > > > > > /** > > > * Check for UDP > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a UDP header > > > + * @retval non-zero if packet contains a UDP header > > > * @retval 0 if packet does not contain a UDP header > > > */ > > > -int odp_packet_has_udp(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_udp(odp_packet_t pkt); > > > > > > /** > > > * Check for TCP > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a TCP header > > > + * @retval non-zero if packet contains a TCP header > > > * @retval 0 if packet does not contain a TCP header > > > */ > > > -int odp_packet_has_tcp(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_tcp(odp_packet_t pkt); > > > > > > /** > > > * Check for SCTP > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains a SCTP header > > > + * @retval non-zero if packet contains a SCTP header > > > * @retval 0 if packet does not contain a SCTP header > > > */ > > > -int odp_packet_has_sctp(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_sctp(odp_packet_t pkt); > > > > > > /** > > > * Check for ICMP > > > * > > > * @param pkt Packet handle > > > - * @retval 1 if packet contains an ICMP header > > > + * @retval non-zero if packet contains an ICMP header > > > * @retval 0 if packet does not contain an ICMP header > > > */ > > > -int odp_packet_has_icmp(odp_packet_t pkt); > > > +odp_bool_t odp_packet_has_icmp(odp_packet_t pkt); > > > > > > /** > > > * Set flag for L2 header, e.g. ethernet > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
