I was referring to flags in a SoC packet descriptor, which may be stored into one or more u64 words. 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]<mailto:[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]<mailto:[email protected]>] Sent: Thursday, March 26, 2015 2:13 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: ext Maxim Uvarov; [email protected]<mailto:[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]<mailto:[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]<mailto:[email protected]>] > Sent: Thursday, March 26, 2015 11:39 AM > To: [email protected]<mailto:[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]<mailto:[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]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
