[2/2] odph_icmp: add ODPH_ prefix

Message ID	1409263329-26011-3-git-send-email-anders.roxell@linaro.org
State	New
Headers	show Return-Path: <patchwork-forward+bncBD5JHFX53YCBBC6O72PQKGQEHNSRPUQ@linaro.org> Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.171 as permitted sender) client-ip=209.85.220.171; Received-SPF: none (google.com: lng-odp-bounces@lists.linaro.org does not designate permitted sender hosts) client-ip=54.225.227.206; From: Anders Roxell <anders.roxell@linaro.org> To: lng-odp@lists.linaro.org Date: Fri, 29 Aug 2014 00:02:09 +0200 Message-Id: <1409263329-26011-3-git-send-email-anders.roxell@linaro.org> In-Reply-To: <1409263329-26011-1-git-send-email-anders.roxell@linaro.org> References: <1409263329-26011-1-git-send-email-anders.roxell@linaro.org> Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix Precedence: list MIME-Version: 1.0 Errors-To: lng-odp-bounces@lists.linaro.org Sender: lng-odp-bounces@lists.linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit

Anders Roxell Aug. 28, 2014, 10:02 p.m. UTC

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 example/generator/odp_generator.c   |  6 +--
 helper/include/odph_chksum.h        |  2 +-
 helper/include/odph_eth.h           |  2 +-
 helper/include/odph_icmp.h          | 79 ++++++++++++++++++-------------------
 helper/include/odph_ip.h            |  2 +-
 helper/include/odph_linux.h         |  2 +-
 helper/include/odph_packet_helper.h |  2 +-
 helper/include/odph_ring.h          |  2 +-
 8 files changed, 48 insertions(+), 49 deletions(-)

Maxim Uvarov Aug. 29, 2014, 10:36 a.m. UTC | #1

I think we do not need to rename defines.  For example

#define ICMP_ECHOREPLY		0	/**< Echo Reply			*/

is always ICMP_ECHOREPLY and is always 0. I suppose that defines were 
copied from
some header file. And my opinion that we should have them as is, not 
implementing new
names.

For helper functions odph_ prefix is ok. But for standard defines it 
look a little ugly.

Best regards,
Maxim.

On 08/29/2014 02:02 AM, Anders Roxell wrote:
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>   example/generator/odp_generator.c   |  6 +--
>   helper/include/odph_chksum.h        |  2 +-
>   helper/include/odph_eth.h           |  2 +-
>   helper/include/odph_icmp.h          | 79 ++++++++++++++++++-------------------
>   helper/include/odph_ip.h            |  2 +-
>   helper/include/odph_linux.h         |  2 +-
>   helper/include/odph_packet_helper.h |  2 +-
>   helper/include/odph_ring.h          |  2 +-
>   8 files changed, 48 insertions(+), 49 deletions(-)
>
> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
> index 70c0353..246bccf 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>   	odph_ipv4_csum_update(pkt);
>   	/* icmp */
>   	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> -	icmp->type = ICMP_ECHO;
> +	icmp->type = ODPH_ICMP_ECHO;
>   	icmp->code = 0;
>   	icmp->un.echo.id = 0;
>   	icmp->un.echo.sequence = ip->id;
> @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], unsigned len)
>   		if (ip->proto == ODPH_IPPROTO_ICMP) {
>   			icmp = (odph_icmphdr_t *)(buf + offset);
>   			/* echo reply */
> -			if (icmp->type == ICMP_ECHOREPLY) {
> +			if (icmp->type == ODPH_ICMP_ECHOREPLY) {
>   				odp_atomic_inc_u64(&counters.icmp);
>   				memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
>   				       sizeof(struct timeval));
> @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], unsigned len)
>   					"ICMP Echo Reply seq %d time %.1f ",
>   					odp_be_to_cpu_16(icmp->un.echo.sequence)
>   					, rtt);
> -			} else if (icmp->type == ICMP_ECHO) {
> +			} else if (icmp->type == ODPH_ICMP_ECHO) {
>   				rlen += sprintf(msg + rlen,
>   						"Icmp Echo Request");
>   			}
> diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
> index 710711a..3bcc2b6 100644
> --- a/helper/include/odph_chksum.h
> +++ b/helper/include/odph_chksum.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * ODP Checksum
> + * ODPH Checksum
>    */
>   #ifndef ODP_CHKSUM_H_
>   #define ODP_CHKSUM_H_
> diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
> index 55a2b1e..45f7fea 100644
> --- a/helper/include/odph_eth.h
> +++ b/helper/include/odph_eth.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * ODP ethernet header
> + * ODPH ethernet header
>    */
>   
>   #ifndef ODPH_ETH_H_
> diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
> index 8414d7e..622f48b 100644
> --- a/helper/include/odph_icmp.h
> +++ b/helper/include/odph_icmp.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * ODP ICMP header
> + * ODPH ICMP header
>    */
>   
>   #ifndef ODPH_ICMP_H_
> @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
>   	} un;			/**< icmp sub header */
>   } odph_icmphdr_t;
>   
> -#define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
> -#define ICMP_DEST_UNREACH	3	/**< Destination Unreachable	*/
> -#define ICMP_SOURCE_QUENCH	4	/**< Source Quench		*/
> -#define ICMP_REDIRECT		5	/**< Redirect (change route)	*/
> -#define ICMP_ECHO		8	/**< Echo Request		*/
> -#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
> -#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
> -#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
> -#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
> -#define ICMP_INFO_REQUEST	15	/**< Information Request	*/
> -#define ICMP_INFO_REPLY		16	/**< Information Reply		*/
> -#define ICMP_ADDRESS		17	/**< Address Mask Request	*/
> -#define ICMP_ADDRESSREPLY	18	/**< Address Mask Reply		*/
> -#define NR_ICMP_TYPES		18	/**< Number of icmp types	*/
> +#define ODPH_ICMP_ECHOREPLY		0  /**< Echo Reply */
> +#define ODPH_ICMP_DEST_UNREACH		3  /**< Destination Unreachable */
> +#define ODPH_ICMP_SOURCE_QUENCH		4  /**< Source Quench */
> +#define ODPH_ICMP_REDIRECT		5  /**< Redirect (change route) */
> +#define ODPH_ICMP_ECHO			8  /**< Echo Request */
> +#define ODPH_ICMP_TIME_EXCEEDED		11 /**< Time Exceeded */
> +#define ODPH_ICMP_PARAMETERPROB		12 /**< Parameter Problem */
> +#define ODPH_ICMP_TIMESTAMP		13 /**< Timestamp Request */
> +#define ODPH_ICMP_TIMESTAMPREPLY	14 /**< Timestamp Reply */
> +#define ODPH_ICMP_INFO_REQUEST		15 /**< Information Request */
> +#define ODPH_ICMP_INFO_REPLY		16 /**< Information Reply */
> +#define ODPH_ICMP_ADDRESS		17 /**< Address Mask Request */
> +#define ODPH_ICMP_ADDRESSREPLY		18 /**< Address Mask Reply */
> +#define ODPH_NR_ICMP_TYPES		18 /**< Number of icmp types */
>   
>   /** Codes for UNREACH. */
> -#define ICMP_NET_UNREACH	0	/**< Network Unreachable	*/
> -#define ICMP_HOST_UNREACH	1	/**< Host Unreachable		*/
> -#define ICMP_PROT_UNREACH	2	/**< Protocol Unreachable	*/
> -#define ICMP_PORT_UNREACH	3	/**< Port Unreachable		*/
> -#define ICMP_FRAG_NEEDED	4	/**< Fragmentation Needed/DF set*/
> -#define ICMP_SR_FAILED		5	/**< Source Route failed	*/
> -#define ICMP_NET_UNKNOWN	6	/**< Network Unknown		*/
> -#define ICMP_HOST_UNKNOWN	7	/**< Host Unknown		*/
> -#define ICMP_HOST_ISOLATED	8	/**< Host Isolated		*/
> -#define ICMP_NET_ANO		9	/**< ICMP_NET_ANO		*/
> -#define ICMP_HOST_ANO		10	/**< ICMP_HOST_ANO		*/
> -#define ICMP_NET_UNR_TOS	11	/**< ICMP_NET_UNR_TOS		*/
> -#define ICMP_HOST_UNR_TOS	12	/**< ICMP_HOST_UNR_TOS		*/
> -#define ICMP_PKT_FILTERED	13	/**< Packet filtered		*/
> -#define ICMP_PREC_VIOLATION	14	/**< Precedence violation	*/
> -#define ICMP_PREC_CUTOFF	15	/**< Precedence cut off		*/
> -#define NR_ICMP_UNREACH		15	/**< instead of hardcoding
> -							immediate value */
> +#define ODPH_ICMP_NET_UNREACH		0  /**< Network Unreachable */
> +#define ODPH_ICMP_HOST_UNREACH		1  /**< Host Unreachable */
> +#define ODPH_ICMP_PROT_UNREACH		2  /**< Protocol Unreachable */
> +#define ODPH_ICMP_PORT_UNREACH		3  /**< Port Unreachable */
> +#define ODPH_ICMP_FRAG_NEEDED		4  /**< Fragmentation Needed/DF set */
> +#define ODPH_ICMP_SR_FAILED		5  /**< Source Route failed */
> +#define ODPH_ICMP_NET_UNKNOWN		6  /**< Network Unknown */
> +#define ODPH_ICMP_HOST_UNKNOWN		7  /**< Host Unknown */
> +#define ODPH_ICMP_HOST_ISOLATED		8  /**< Host Isolated */
> +#define ODPH_ICMP_NET_ANO		9  /**< ICMP_NET_ANO */
> +#define ODPH_ICMP_HOST_ANO		10 /**< ICMP_HOST_ANO */
> +#define ODPH_ICMP_NET_UNR_TOS		11 /**< ICMP_NET_UNR_TOS */
> +#define ODPH_ICMP_HOST_UNR_TOS		12 /**< ICMP_HOST_UNR_TOS */
> +#define ODPH_ICMP_PKT_FILTERED		13 /**< Packet filtered */
> +#define ODPH_ICMP_PREC_VIOLATION	14 /**< Precedence violation */
> +#define ODPH_ICMP_PREC_CUTOFF		15 /**< Precedence cut off */
> +#define ODPH_NR_ICMP_UNREACH		15 /**< instead of hardcoding
> +						immediate value */
>   
>   /** Codes for REDIRECT. */
> -#define ICMP_REDIR_NET		0	/**< Redirect Net		*/
> -#define ICMP_REDIR_HOST		1	/**< Redirect Host		*/
> -#define ICMP_REDIR_NETTOS	2	/**< Redirect Net for TOS	*/
> -#define ICMP_REDIR_HOSTTOS	3	/**< Redirect Host for TOS	*/
> +#define ODPH_ICMP_REDIR_NET		0  /**< Redirect Net */
> +#define ODPH_ICMP_REDIR_HOST		1  /**< Redirect Host */
> +#define ODPH_ICMP_REDIR_NETTOS		2  /**< Redirect Net for TOS */
> +#define ODPH_ICMP_REDIR_HOSTTOS		3  /**< Redirect Host for TOS */
>   
>   /** Codes for TIME_EXCEEDED. */
> -#define ICMP_EXC_TTL		0	/**< TTL count exceeded		*/
> -#define ICMP_EXC_FRAGTIME	1	/**< Fragment Reass time
> -								exceeded*/
> +#define ODPH_ICMP_EXC_TTL		0  /**< TTL count exceeded */
> +#define ODPH_ICMP_EXC_FRAGTIME		1  /**< Fragment Reass time exceeded*/
>   
>   /** @internal Compile time assert */
>   ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN, "ODPH_ICMPHDR_T__SIZE_ERROR");
> diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> index ca71c44..4fb9c12 100644
> --- a/helper/include/odph_ip.h
> +++ b/helper/include/odph_ip.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * ODP IP header
> + * ODPH IP header
>    */
>   
>   #ifndef ODPH_IP_H_
> diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
> index 1ea349a..be91dd3 100644
> --- a/helper/include/odph_linux.h
> +++ b/helper/include/odph_linux.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * ODP Linux helper API
> + * ODPH Linux helper API
>    *
>    * This file is an optional helper to odp.h APIs. Application can manage
>    * pthreads also by other means.
> diff --git a/helper/include/odph_packet_helper.h b/helper/include/odph_packet_helper.h
> index c18f48d..c5315a9 100644
> --- a/helper/include/odph_packet_helper.h
> +++ b/helper/include/odph_packet_helper.h
> @@ -8,7 +8,7 @@
>   /**
>    * @file
>    *
> - * Optional ODP packet helper functions
> + * Optional ODPH packet helper functions
>    */
>   
>   #ifndef ODPH_PACKET_HELPER_H_
> diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
> index 76c1db8..82d2d13 100644
> --- a/helper/include/odph_ring.h
> +++ b/helper/include/odph_ring.h
> @@ -192,7 +192,7 @@ typedef struct odph_ring {
>    *    - ENOMEM - no appropriate memory area found in which to create memzone
>    */
>   odph_ring_t *odph_ring_create(const char *name, unsigned count,
> -			    unsigned flags);
> +			      unsigned flags);
>   
>   
>   /**

Savolainen, Petri (NSN - FI/Espoo) Aug. 29, 2014, 10:53 a.m. UTC | #2

Hi,

ODP/ODPH prefixes are needed always. First to make it clear what comes ODP/ODP helper, and secondly to avoid name space collisions (e.g. with linux/posix/application files that define similar things).

-Petri


> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
> Sent: Friday, August 29, 2014 1:36 PM
> To: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> 
> I think we do not need to rename defines.  For example
> 
> #define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
> 
> is always ICMP_ECHOREPLY and is always 0. I suppose that defines were
> copied from
> some header file. And my opinion that we should have them as is, not
> implementing new
> names.
> 
> For helper functions odph_ prefix is ok. But for standard defines it
> look a little ugly.
> 
> Best regards,
> Maxim.
> 
> On 08/29/2014 02:02 AM, Anders Roxell wrote:
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> >   example/generator/odp_generator.c   |  6 +--
> >   helper/include/odph_chksum.h        |  2 +-
> >   helper/include/odph_eth.h           |  2 +-
> >   helper/include/odph_icmp.h          | 79 ++++++++++++++++++-----------
> --------
> >   helper/include/odph_ip.h            |  2 +-
> >   helper/include/odph_linux.h         |  2 +-
> >   helper/include/odph_packet_helper.h |  2 +-
> >   helper/include/odph_ring.h          |  2 +-
> >   8 files changed, 48 insertions(+), 49 deletions(-)
> >
> > diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> > index 70c0353..246bccf 100644
> > --- a/example/generator/odp_generator.c
> > +++ b/example/generator/odp_generator.c
> > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
> >   	odph_ipv4_csum_update(pkt);
> >   	/* icmp */
> >   	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> > -	icmp->type = ICMP_ECHO;
> > +	icmp->type = ODPH_ICMP_ECHO;
> >   	icmp->code = 0;
> >   	icmp->un.echo.id = 0;
> >   	icmp->un.echo.sequence = ip->id;
> > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
> pkt_tbl[], unsigned len)
> >   		if (ip->proto == ODPH_IPPROTO_ICMP) {
> >   			icmp = (odph_icmphdr_t *)(buf + offset);
> >   			/* echo reply */
> > -			if (icmp->type == ICMP_ECHOREPLY) {
> > +			if (icmp->type == ODPH_ICMP_ECHOREPLY) {
> >   				odp_atomic_inc_u64(&counters.icmp);
> >   				memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
> >   				       sizeof(struct timeval));
> > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
> pkt_tbl[], unsigned len)
> >   					"ICMP Echo Reply seq %d time %.1f ",
> >   					odp_be_to_cpu_16(icmp->un.echo.sequence)
> >   					, rtt);
> > -			} else if (icmp->type == ICMP_ECHO) {
> > +			} else if (icmp->type == ODPH_ICMP_ECHO) {
> >   				rlen += sprintf(msg + rlen,
> >   						"Icmp Echo Request");
> >   			}
> > diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
> > index 710711a..3bcc2b6 100644
> > --- a/helper/include/odph_chksum.h
> > +++ b/helper/include/odph_chksum.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * ODP Checksum
> > + * ODPH Checksum
> >    */
> >   #ifndef ODP_CHKSUM_H_
> >   #define ODP_CHKSUM_H_
> > diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
> > index 55a2b1e..45f7fea 100644
> > --- a/helper/include/odph_eth.h
> > +++ b/helper/include/odph_eth.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * ODP ethernet header
> > + * ODPH ethernet header
> >    */
> >
> >   #ifndef ODPH_ETH_H_
> > diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
> > index 8414d7e..622f48b 100644
> > --- a/helper/include/odph_icmp.h
> > +++ b/helper/include/odph_icmp.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * ODP ICMP header
> > + * ODPH ICMP header
> >    */
> >
> >   #ifndef ODPH_ICMP_H_
> > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
> >   	} un;			/**< icmp sub header */
> >   } odph_icmphdr_t;
> >
> > -#define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
> > -#define ICMP_DEST_UNREACH	3	/**< Destination Unreachable	*/
> > -#define ICMP_SOURCE_QUENCH	4	/**< Source Quench		*/
> > -#define ICMP_REDIRECT		5	/**< Redirect (change route)	*/
> > -#define ICMP_ECHO		8	/**< Echo Request		*/
> > -#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
> > -#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
> > -#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
> > -#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
> > -#define ICMP_INFO_REQUEST	15	/**< Information Request	*/
> > -#define ICMP_INFO_REPLY		16	/**< Information Reply		*/
> > -#define ICMP_ADDRESS		17	/**< Address Mask Request	*/
> > -#define ICMP_ADDRESSREPLY	18	/**< Address Mask Reply		*/
> > -#define NR_ICMP_TYPES		18	/**< Number of icmp types	*/
> > +#define ODPH_ICMP_ECHOREPLY		0  /**< Echo Reply */
> > +#define ODPH_ICMP_DEST_UNREACH		3  /**< Destination Unreachable
> */
> > +#define ODPH_ICMP_SOURCE_QUENCH		4  /**< Source Quench */
> > +#define ODPH_ICMP_REDIRECT		5  /**< Redirect (change route) */
> > +#define ODPH_ICMP_ECHO			8  /**< Echo Request */
> > +#define ODPH_ICMP_TIME_EXCEEDED		11 /**< Time Exceeded */
> > +#define ODPH_ICMP_PARAMETERPROB		12 /**< Parameter Problem */
> > +#define ODPH_ICMP_TIMESTAMP		13 /**< Timestamp Request */
> > +#define ODPH_ICMP_TIMESTAMPREPLY	14 /**< Timestamp Reply */
> > +#define ODPH_ICMP_INFO_REQUEST		15 /**< Information Request */
> > +#define ODPH_ICMP_INFO_REPLY		16 /**< Information Reply */
> > +#define ODPH_ICMP_ADDRESS		17 /**< Address Mask Request */
> > +#define ODPH_ICMP_ADDRESSREPLY		18 /**< Address Mask Reply */
> > +#define ODPH_NR_ICMP_TYPES		18 /**< Number of icmp types */
> >
> >   /** Codes for UNREACH. */
> > -#define ICMP_NET_UNREACH	0	/**< Network Unreachable	*/
> > -#define ICMP_HOST_UNREACH	1	/**< Host Unreachable		*/
> > -#define ICMP_PROT_UNREACH	2	/**< Protocol Unreachable	*/
> > -#define ICMP_PORT_UNREACH	3	/**< Port Unreachable		*/
> > -#define ICMP_FRAG_NEEDED	4	/**< Fragmentation Needed/DF set*/
> > -#define ICMP_SR_FAILED		5	/**< Source Route failed	*/
> > -#define ICMP_NET_UNKNOWN	6	/**< Network Unknown		*/
> > -#define ICMP_HOST_UNKNOWN	7	/**< Host Unknown		*/
> > -#define ICMP_HOST_ISOLATED	8	/**< Host Isolated		*/
> > -#define ICMP_NET_ANO		9	/**< ICMP_NET_ANO		*/
> > -#define ICMP_HOST_ANO		10	/**< ICMP_HOST_ANO		*/
> > -#define ICMP_NET_UNR_TOS	11	/**< ICMP_NET_UNR_TOS		*/
> > -#define ICMP_HOST_UNR_TOS	12	/**< ICMP_HOST_UNR_TOS		*/
> > -#define ICMP_PKT_FILTERED	13	/**< Packet filtered		*/
> > -#define ICMP_PREC_VIOLATION	14	/**< Precedence violation	*/
> > -#define ICMP_PREC_CUTOFF	15	/**< Precedence cut off		*/
> > -#define NR_ICMP_UNREACH		15	/**< instead of hardcoding
> > -							immediate value */
> > +#define ODPH_ICMP_NET_UNREACH		0  /**< Network Unreachable */
> > +#define ODPH_ICMP_HOST_UNREACH		1  /**< Host Unreachable */
> > +#define ODPH_ICMP_PROT_UNREACH		2  /**< Protocol Unreachable */
> > +#define ODPH_ICMP_PORT_UNREACH		3  /**< Port Unreachable */
> > +#define ODPH_ICMP_FRAG_NEEDED		4  /**< Fragmentation Needed/DF
> set */
> > +#define ODPH_ICMP_SR_FAILED		5  /**< Source Route failed */
> > +#define ODPH_ICMP_NET_UNKNOWN		6  /**< Network Unknown */
> > +#define ODPH_ICMP_HOST_UNKNOWN		7  /**< Host Unknown */
> > +#define ODPH_ICMP_HOST_ISOLATED		8  /**< Host Isolated */
> > +#define ODPH_ICMP_NET_ANO		9  /**< ICMP_NET_ANO */
> > +#define ODPH_ICMP_HOST_ANO		10 /**< ICMP_HOST_ANO */
> > +#define ODPH_ICMP_NET_UNR_TOS		11 /**< ICMP_NET_UNR_TOS */
> > +#define ODPH_ICMP_HOST_UNR_TOS		12 /**< ICMP_HOST_UNR_TOS */
> > +#define ODPH_ICMP_PKT_FILTERED		13 /**< Packet filtered */
> > +#define ODPH_ICMP_PREC_VIOLATION	14 /**< Precedence violation */
> > +#define ODPH_ICMP_PREC_CUTOFF		15 /**< Precedence cut off */
> > +#define ODPH_NR_ICMP_UNREACH		15 /**< instead of hardcoding
> > +						immediate value */
> >
> >   /** Codes for REDIRECT. */
> > -#define ICMP_REDIR_NET		0	/**< Redirect Net		*/
> > -#define ICMP_REDIR_HOST		1	/**< Redirect Host		*/
> > -#define ICMP_REDIR_NETTOS	2	/**< Redirect Net for TOS	*/
> > -#define ICMP_REDIR_HOSTTOS	3	/**< Redirect Host for TOS	*/
> > +#define ODPH_ICMP_REDIR_NET		0  /**< Redirect Net */
> > +#define ODPH_ICMP_REDIR_HOST		1  /**< Redirect Host */
> > +#define ODPH_ICMP_REDIR_NETTOS		2  /**< Redirect Net for TOS */
> > +#define ODPH_ICMP_REDIR_HOSTTOS		3  /**< Redirect Host for TOS */
> >
> >   /** Codes for TIME_EXCEEDED. */
> > -#define ICMP_EXC_TTL		0	/**< TTL count exceeded		*/
> > -#define ICMP_EXC_FRAGTIME	1	/**< Fragment Reass time
> > -								exceeded*/
> > +#define ODPH_ICMP_EXC_TTL		0  /**< TTL count exceeded */
> > +#define ODPH_ICMP_EXC_FRAGTIME		1  /**< Fragment Reass time
> exceeded*/
> >
> >   /** @internal Compile time assert */
> >   ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
> "ODPH_ICMPHDR_T__SIZE_ERROR");
> > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> > index ca71c44..4fb9c12 100644
> > --- a/helper/include/odph_ip.h
> > +++ b/helper/include/odph_ip.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * ODP IP header
> > + * ODPH IP header
> >    */
> >
> >   #ifndef ODPH_IP_H_
> > diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
> > index 1ea349a..be91dd3 100644
> > --- a/helper/include/odph_linux.h
> > +++ b/helper/include/odph_linux.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * ODP Linux helper API
> > + * ODPH Linux helper API
> >    *
> >    * This file is an optional helper to odp.h APIs. Application can
> manage
> >    * pthreads also by other means.
> > diff --git a/helper/include/odph_packet_helper.h
> b/helper/include/odph_packet_helper.h
> > index c18f48d..c5315a9 100644
> > --- a/helper/include/odph_packet_helper.h
> > +++ b/helper/include/odph_packet_helper.h
> > @@ -8,7 +8,7 @@
> >   /**
> >    * @file
> >    *
> > - * Optional ODP packet helper functions
> > + * Optional ODPH packet helper functions
> >    */
> >
> >   #ifndef ODPH_PACKET_HELPER_H_
> > diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
> > index 76c1db8..82d2d13 100644
> > --- a/helper/include/odph_ring.h
> > +++ b/helper/include/odph_ring.h
> > @@ -192,7 +192,7 @@ typedef struct odph_ring {
> >    *    - ENOMEM - no appropriate memory area found in which to create
> memzone
> >    */
> >   odph_ring_t *odph_ring_create(const char *name, unsigned count,
> > -			    unsigned flags);
> > +			      unsigned flags);
> >
> >
> >   /**
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Mike Holmes Aug. 29, 2014, 11:28 a.m. UTC | #3

On 29 August 2014 06:53, Savolainen, Petri (NSN - FI/Espoo) <
petri.savolainen@nsn.com> wrote:

> Hi,
>
> ODP/ODPH prefixes are needed always. First to make it clear what comes
> ODP/ODP helper, and secondly to avoid name space collisions (e.g. with
> linux/posix/application files that define similar things).
>
> -Petri
>
>
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
> > Sent: Friday, August 29, 2014 1:36 PM
> > To: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> >
> > I think we do not need to rename defines.  For example
> >
> > #define ICMP_ECHOREPLY                0       /**< Echo Reply
>      */
> >
> > is always ICMP_ECHOREPLY and is always 0. I suppose that defines were
> > copied from
> > some header file. And my opinion that we should have them as is, not
> > implementing new
> > names.
> >
> > For helper functions odph_ prefix is ok. But for standard defines it
> > look a little ugly.
> >
> > Best regards,
> > Maxim.
> >
> > On 08/29/2014 02:02 AM, Anders Roxell wrote:
> > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>
Reviewed-by:Mike Holmes <mike.holmes@linaro.org>

> > > ---
> > >   example/generator/odp_generator.c   |  6 +--
> > >   helper/include/odph_chksum.h        |  2 +-
> > >   helper/include/odph_eth.h           |  2 +-
> > >   helper/include/odph_icmp.h          | 79
> ++++++++++++++++++-----------
> > --------
> > >   helper/include/odph_ip.h            |  2 +-
> > >   helper/include/odph_linux.h         |  2 +-
> > >   helper/include/odph_packet_helper.h |  2 +-
> > >   helper/include/odph_ring.h          |  2 +-
> > >   8 files changed, 48 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/example/generator/odp_generator.c
> > b/example/generator/odp_generator.c
> > > index 70c0353..246bccf 100644
> > > --- a/example/generator/odp_generator.c
> > > +++ b/example/generator/odp_generator.c
> > > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
> > >     odph_ipv4_csum_update(pkt);
> > >     /* icmp */
> > >     icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +
> ODPH_IPV4HDR_LEN);
> > > -   icmp->type = ICMP_ECHO;
> > > +   icmp->type = ODPH_ICMP_ECHO;
> > >     icmp->code = 0;
> > >     icmp->un.echo.id = 0;
> > >     icmp->un.echo.sequence = ip->id;
> > > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
> > pkt_tbl[], unsigned len)
> > >             if (ip->proto == ODPH_IPPROTO_ICMP) {
> > >                     icmp = (odph_icmphdr_t *)(buf + offset);
> > >                     /* echo reply */
> > > -                   if (icmp->type == ICMP_ECHOREPLY) {
> > > +                   if (icmp->type == ODPH_ICMP_ECHOREPLY) {
> > >                             odp_atomic_inc_u64(&counters.icmp);
> > >                             memcpy(&tvsend, buf + offset +
> ODPH_ICMPHDR_LEN,
> > >                                    sizeof(struct timeval));
> > > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
> > pkt_tbl[], unsigned len)
> > >                                     "ICMP Echo Reply seq %d time %.1f
> ",
> > >
>  odp_be_to_cpu_16(icmp->un.echo.sequence)
> > >                                     , rtt);
> > > -                   } else if (icmp->type == ICMP_ECHO) {
> > > +                   } else if (icmp->type == ODPH_ICMP_ECHO) {
> > >                             rlen += sprintf(msg + rlen,
> > >                                             "Icmp Echo Request");
> > >                     }
> > > diff --git a/helper/include/odph_chksum.h
> b/helper/include/odph_chksum.h
> > > index 710711a..3bcc2b6 100644
> > > --- a/helper/include/odph_chksum.h
> > > +++ b/helper/include/odph_chksum.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * ODP Checksum
> > > + * ODPH Checksum
> > >    */
> > >   #ifndef ODP_CHKSUM_H_
> > >   #define ODP_CHKSUM_H_
> > > diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
> > > index 55a2b1e..45f7fea 100644
> > > --- a/helper/include/odph_eth.h
> > > +++ b/helper/include/odph_eth.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * ODP ethernet header
> > > + * ODPH ethernet header
> > >    */
> > >
> > >   #ifndef ODPH_ETH_H_
> > > diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
> > > index 8414d7e..622f48b 100644
> > > --- a/helper/include/odph_icmp.h
> > > +++ b/helper/include/odph_icmp.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * ODP ICMP header
> > > + * ODPH ICMP header
> > >    */
> > >
> > >   #ifndef ODPH_ICMP_H_
> > > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
> > >     } un;                   /**< icmp sub header */
> > >   } odph_icmphdr_t;
> > >
> > > -#define ICMP_ECHOREPLY             0       /**< Echo Reply
>      */
> > > -#define ICMP_DEST_UNREACH  3       /**< Destination Unreachable    */
> > > -#define ICMP_SOURCE_QUENCH 4       /**< Source Quench              */
> > > -#define ICMP_REDIRECT              5       /**< Redirect (change
> route)    */
> > > -#define ICMP_ECHO          8       /**< Echo Request               */
> > > -#define ICMP_TIME_EXCEEDED 11      /**< Time Exceeded              */
> > > -#define ICMP_PARAMETERPROB 12      /**< Parameter Problem          */
> > > -#define ICMP_TIMESTAMP             13      /**< Timestamp Request
>       */
> > > -#define ICMP_TIMESTAMPREPLY        14      /**< Timestamp Reply
>       */
> > > -#define ICMP_INFO_REQUEST  15      /**< Information Request        */
> > > -#define ICMP_INFO_REPLY            16      /**< Information Reply
>       */
> > > -#define ICMP_ADDRESS               17      /**< Address Mask Request
>      */
> > > -#define ICMP_ADDRESSREPLY  18      /**< Address Mask Reply         */
> > > -#define NR_ICMP_TYPES              18      /**< Number of icmp types
>      */
> > > +#define ODPH_ICMP_ECHOREPLY                0  /**< Echo Reply */
> > > +#define ODPH_ICMP_DEST_UNREACH             3  /**< Destination
> Unreachable
> > */
> > > +#define ODPH_ICMP_SOURCE_QUENCH            4  /**< Source Quench */
> > > +#define ODPH_ICMP_REDIRECT         5  /**< Redirect (change route) */
> > > +#define ODPH_ICMP_ECHO                     8  /**< Echo Request */
> > > +#define ODPH_ICMP_TIME_EXCEEDED            11 /**< Time Exceeded */
> > > +#define ODPH_ICMP_PARAMETERPROB            12 /**< Parameter Problem
> */
> > > +#define ODPH_ICMP_TIMESTAMP                13 /**< Timestamp Request
> */
> > > +#define ODPH_ICMP_TIMESTAMPREPLY   14 /**< Timestamp Reply */
> > > +#define ODPH_ICMP_INFO_REQUEST             15 /**< Information
> Request */
> > > +#define ODPH_ICMP_INFO_REPLY               16 /**< Information Reply
> */
> > > +#define ODPH_ICMP_ADDRESS          17 /**< Address Mask Request */
> > > +#define ODPH_ICMP_ADDRESSREPLY             18 /**< Address Mask Reply
> */
> > > +#define ODPH_NR_ICMP_TYPES         18 /**< Number of icmp types */
> > >
> > >   /** Codes for UNREACH. */
> > > -#define ICMP_NET_UNREACH   0       /**< Network Unreachable        */
> > > -#define ICMP_HOST_UNREACH  1       /**< Host Unreachable           */
> > > -#define ICMP_PROT_UNREACH  2       /**< Protocol Unreachable       */
> > > -#define ICMP_PORT_UNREACH  3       /**< Port Unreachable           */
> > > -#define ICMP_FRAG_NEEDED   4       /**< Fragmentation Needed/DF set*/
> > > -#define ICMP_SR_FAILED             5       /**< Source Route failed
>       */
> > > -#define ICMP_NET_UNKNOWN   6       /**< Network Unknown            */
> > > -#define ICMP_HOST_UNKNOWN  7       /**< Host Unknown               */
> > > -#define ICMP_HOST_ISOLATED 8       /**< Host Isolated              */
> > > -#define ICMP_NET_ANO               9       /**< ICMP_NET_ANO
>      */
> > > -#define ICMP_HOST_ANO              10      /**< ICMP_HOST_ANO
>       */
> > > -#define ICMP_NET_UNR_TOS   11      /**< ICMP_NET_UNR_TOS           */
> > > -#define ICMP_HOST_UNR_TOS  12      /**< ICMP_HOST_UNR_TOS          */
> > > -#define ICMP_PKT_FILTERED  13      /**< Packet filtered            */
> > > -#define ICMP_PREC_VIOLATION        14      /**< Precedence violation
>      */
> > > -#define ICMP_PREC_CUTOFF   15      /**< Precedence cut off         */
> > > -#define NR_ICMP_UNREACH            15      /**< instead of hardcoding
> > > -                                                   immediate value */
> > > +#define ODPH_ICMP_NET_UNREACH              0  /**< Network
> Unreachable */
> > > +#define ODPH_ICMP_HOST_UNREACH             1  /**< Host Unreachable */
> > > +#define ODPH_ICMP_PROT_UNREACH             2  /**< Protocol
> Unreachable */
> > > +#define ODPH_ICMP_PORT_UNREACH             3  /**< Port Unreachable */
> > > +#define ODPH_ICMP_FRAG_NEEDED              4  /**< Fragmentation
> Needed/DF
> > set */
> > > +#define ODPH_ICMP_SR_FAILED                5  /**< Source Route
> failed */
> > > +#define ODPH_ICMP_NET_UNKNOWN              6  /**< Network Unknown */
> > > +#define ODPH_ICMP_HOST_UNKNOWN             7  /**< Host Unknown */
> > > +#define ODPH_ICMP_HOST_ISOLATED            8  /**< Host Isolated */
> > > +#define ODPH_ICMP_NET_ANO          9  /**< ICMP_NET_ANO */
> > > +#define ODPH_ICMP_HOST_ANO         10 /**< ICMP_HOST_ANO */
> > > +#define ODPH_ICMP_NET_UNR_TOS              11 /**< ICMP_NET_UNR_TOS */
> > > +#define ODPH_ICMP_HOST_UNR_TOS             12 /**< ICMP_HOST_UNR_TOS
> */
> > > +#define ODPH_ICMP_PKT_FILTERED             13 /**< Packet filtered */
> > > +#define ODPH_ICMP_PREC_VIOLATION   14 /**< Precedence violation */
> > > +#define ODPH_ICMP_PREC_CUTOFF              15 /**< Precedence cut off
> */
> > > +#define ODPH_NR_ICMP_UNREACH               15 /**< instead of
> hardcoding
> > > +                                           immediate value */
> > >
> > >   /** Codes for REDIRECT. */
> > > -#define ICMP_REDIR_NET             0       /**< Redirect Net
>      */
> > > -#define ICMP_REDIR_HOST            1       /**< Redirect Host
>       */
> > > -#define ICMP_REDIR_NETTOS  2       /**< Redirect Net for TOS       */
> > > -#define ICMP_REDIR_HOSTTOS 3       /**< Redirect Host for TOS      */
> > > +#define ODPH_ICMP_REDIR_NET                0  /**< Redirect Net */
> > > +#define ODPH_ICMP_REDIR_HOST               1  /**< Redirect Host */
> > > +#define ODPH_ICMP_REDIR_NETTOS             2  /**< Redirect Net for
> TOS */
> > > +#define ODPH_ICMP_REDIR_HOSTTOS            3  /**< Redirect Host for
> TOS */
> > >
> > >   /** Codes for TIME_EXCEEDED. */
> > > -#define ICMP_EXC_TTL               0       /**< TTL count exceeded
>      */
> > > -#define ICMP_EXC_FRAGTIME  1       /**< Fragment Reass time
> > > -                                                           exceeded*/
> > > +#define ODPH_ICMP_EXC_TTL          0  /**< TTL count exceeded */
> > > +#define ODPH_ICMP_EXC_FRAGTIME             1  /**< Fragment Reass time
> > exceeded*/
> > >
> > >   /** @internal Compile time assert */
> > >   ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
> > "ODPH_ICMPHDR_T__SIZE_ERROR");
> > > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> > > index ca71c44..4fb9c12 100644
> > > --- a/helper/include/odph_ip.h
> > > +++ b/helper/include/odph_ip.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * ODP IP header
> > > + * ODPH IP header
> > >    */
> > >
> > >   #ifndef ODPH_IP_H_
> > > diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
> > > index 1ea349a..be91dd3 100644
> > > --- a/helper/include/odph_linux.h
> > > +++ b/helper/include/odph_linux.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * ODP Linux helper API
> > > + * ODPH Linux helper API
> > >    *
> > >    * This file is an optional helper to odp.h APIs. Application can
> > manage
> > >    * pthreads also by other means.
> > > diff --git a/helper/include/odph_packet_helper.h
> > b/helper/include/odph_packet_helper.h
> > > index c18f48d..c5315a9 100644
> > > --- a/helper/include/odph_packet_helper.h
> > > +++ b/helper/include/odph_packet_helper.h
> > > @@ -8,7 +8,7 @@
> > >   /**
> > >    * @file
> > >    *
> > > - * Optional ODP packet helper functions
> > > + * Optional ODPH packet helper functions
> > >    */
> > >
> > >   #ifndef ODPH_PACKET_HELPER_H_
> > > diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
> > > index 76c1db8..82d2d13 100644
> > > --- a/helper/include/odph_ring.h
> > > +++ b/helper/include/odph_ring.h
> > > @@ -192,7 +192,7 @@ typedef struct odph_ring {
> > >    *    - ENOMEM - no appropriate memory area found in which to create
> > memzone
> > >    */
> > >   odph_ring_t *odph_ring_create(const char *name, unsigned count,
> > > -                       unsigned flags);
> > > +                         unsigned flags);
> > >
> > >
> > >   /**
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

Savolainen, Petri (NSN - FI/Espoo) Aug. 29, 2014, 11:49 a.m. UTC | #4

> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Anders Roxell
> Sent: Friday, August 29, 2014 1:02 AM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> 
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Acked-by: Petri Savolainen <petri.savolainen@linaro.org>
> ---
>  example/generator/odp_generator.c   |  6 +--
>  helper/include/odph_chksum.h        |  2 +-
>  helper/include/odph_eth.h           |  2 +-
>  helper/include/odph_icmp.h          | 79 ++++++++++++++++++--------------
> -----
>  helper/include/odph_ip.h            |  2 +-
>  helper/include/odph_linux.h         |  2 +-
>  helper/include/odph_packet_helper.h |  2 +-
>  helper/include/odph_ring.h          |  2 +-
>  8 files changed, 48 insertions(+), 49 deletions(-)
> 
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index 70c0353..246bccf 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>  	odph_ipv4_csum_update(pkt);
>  	/* icmp */
>  	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> -	icmp->type = ICMP_ECHO;
> +	icmp->type = ODPH_ICMP_ECHO;
>  	icmp->code = 0;
>  	icmp->un.echo.id = 0;
>  	icmp->un.echo.sequence = ip->id;
> @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
> pkt_tbl[], unsigned len)
>  		if (ip->proto == ODPH_IPPROTO_ICMP) {
>  			icmp = (odph_icmphdr_t *)(buf + offset);
>  			/* echo reply */
> -			if (icmp->type == ICMP_ECHOREPLY) {
> +			if (icmp->type == ODPH_ICMP_ECHOREPLY) {
>  				odp_atomic_inc_u64(&counters.icmp);
>  				memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
>  				       sizeof(struct timeval));
> @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
> pkt_tbl[], unsigned len)
>  					"ICMP Echo Reply seq %d time %.1f ",
>  					odp_be_to_cpu_16(icmp->un.echo.sequence)
>  					, rtt);
> -			} else if (icmp->type == ICMP_ECHO) {
> +			} else if (icmp->type == ODPH_ICMP_ECHO) {
>  				rlen += sprintf(msg + rlen,
>  						"Icmp Echo Request");
>  			}
> diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
> index 710711a..3bcc2b6 100644
> --- a/helper/include/odph_chksum.h
> +++ b/helper/include/odph_chksum.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * ODP Checksum
> + * ODPH Checksum
>   */
>  #ifndef ODP_CHKSUM_H_
>  #define ODP_CHKSUM_H_
> diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
> index 55a2b1e..45f7fea 100644
> --- a/helper/include/odph_eth.h
> +++ b/helper/include/odph_eth.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * ODP ethernet header
> + * ODPH ethernet header
>   */
> 
>  #ifndef ODPH_ETH_H_
> diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
> index 8414d7e..622f48b 100644
> --- a/helper/include/odph_icmp.h
> +++ b/helper/include/odph_icmp.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * ODP ICMP header
> + * ODPH ICMP header
>   */
> 
>  #ifndef ODPH_ICMP_H_
> @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
>  	} un;			/**< icmp sub header */
>  } odph_icmphdr_t;
> 
> -#define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
> -#define ICMP_DEST_UNREACH	3	/**< Destination Unreachable	*/
> -#define ICMP_SOURCE_QUENCH	4	/**< Source Quench		*/
> -#define ICMP_REDIRECT		5	/**< Redirect (change route)	*/
> -#define ICMP_ECHO		8	/**< Echo Request		*/
> -#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
> -#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
> -#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
> -#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
> -#define ICMP_INFO_REQUEST	15	/**< Information Request	*/
> -#define ICMP_INFO_REPLY		16	/**< Information Reply		*/
> -#define ICMP_ADDRESS		17	/**< Address Mask Request	*/
> -#define ICMP_ADDRESSREPLY	18	/**< Address Mask Reply		*/
> -#define NR_ICMP_TYPES		18	/**< Number of icmp types	*/
> +#define ODPH_ICMP_ECHOREPLY		0  /**< Echo Reply */
> +#define ODPH_ICMP_DEST_UNREACH		3  /**< Destination Unreachable
> */
> +#define ODPH_ICMP_SOURCE_QUENCH		4  /**< Source Quench */
> +#define ODPH_ICMP_REDIRECT		5  /**< Redirect (change route) */
> +#define ODPH_ICMP_ECHO			8  /**< Echo Request */
> +#define ODPH_ICMP_TIME_EXCEEDED		11 /**< Time Exceeded */
> +#define ODPH_ICMP_PARAMETERPROB		12 /**< Parameter Problem */
> +#define ODPH_ICMP_TIMESTAMP		13 /**< Timestamp Request */
> +#define ODPH_ICMP_TIMESTAMPREPLY	14 /**< Timestamp Reply */
> +#define ODPH_ICMP_INFO_REQUEST		15 /**< Information Request */
> +#define ODPH_ICMP_INFO_REPLY		16 /**< Information Reply */
> +#define ODPH_ICMP_ADDRESS		17 /**< Address Mask Request */
> +#define ODPH_ICMP_ADDRESSREPLY		18 /**< Address Mask Reply */
> +#define ODPH_NR_ICMP_TYPES		18 /**< Number of icmp types */
> 
>  /** Codes for UNREACH. */
> -#define ICMP_NET_UNREACH	0	/**< Network Unreachable	*/
> -#define ICMP_HOST_UNREACH	1	/**< Host Unreachable		*/
> -#define ICMP_PROT_UNREACH	2	/**< Protocol Unreachable	*/
> -#define ICMP_PORT_UNREACH	3	/**< Port Unreachable		*/
> -#define ICMP_FRAG_NEEDED	4	/**< Fragmentation Needed/DF set*/
> -#define ICMP_SR_FAILED		5	/**< Source Route failed	*/
> -#define ICMP_NET_UNKNOWN	6	/**< Network Unknown		*/
> -#define ICMP_HOST_UNKNOWN	7	/**< Host Unknown		*/
> -#define ICMP_HOST_ISOLATED	8	/**< Host Isolated		*/
> -#define ICMP_NET_ANO		9	/**< ICMP_NET_ANO		*/
> -#define ICMP_HOST_ANO		10	/**< ICMP_HOST_ANO		*/
> -#define ICMP_NET_UNR_TOS	11	/**< ICMP_NET_UNR_TOS		*/
> -#define ICMP_HOST_UNR_TOS	12	/**< ICMP_HOST_UNR_TOS		*/
> -#define ICMP_PKT_FILTERED	13	/**< Packet filtered		*/
> -#define ICMP_PREC_VIOLATION	14	/**< Precedence violation	*/
> -#define ICMP_PREC_CUTOFF	15	/**< Precedence cut off		*/
> -#define NR_ICMP_UNREACH		15	/**< instead of hardcoding
> -							immediate value */
> +#define ODPH_ICMP_NET_UNREACH		0  /**< Network Unreachable */
> +#define ODPH_ICMP_HOST_UNREACH		1  /**< Host Unreachable */
> +#define ODPH_ICMP_PROT_UNREACH		2  /**< Protocol Unreachable */
> +#define ODPH_ICMP_PORT_UNREACH		3  /**< Port Unreachable */
> +#define ODPH_ICMP_FRAG_NEEDED		4  /**< Fragmentation Needed/DF
> set */
> +#define ODPH_ICMP_SR_FAILED		5  /**< Source Route failed */
> +#define ODPH_ICMP_NET_UNKNOWN		6  /**< Network Unknown */
> +#define ODPH_ICMP_HOST_UNKNOWN		7  /**< Host Unknown */
> +#define ODPH_ICMP_HOST_ISOLATED		8  /**< Host Isolated */
> +#define ODPH_ICMP_NET_ANO		9  /**< ICMP_NET_ANO */
> +#define ODPH_ICMP_HOST_ANO		10 /**< ICMP_HOST_ANO */
> +#define ODPH_ICMP_NET_UNR_TOS		11 /**< ICMP_NET_UNR_TOS */
> +#define ODPH_ICMP_HOST_UNR_TOS		12 /**< ICMP_HOST_UNR_TOS */
> +#define ODPH_ICMP_PKT_FILTERED		13 /**< Packet filtered */
> +#define ODPH_ICMP_PREC_VIOLATION	14 /**< Precedence violation */
> +#define ODPH_ICMP_PREC_CUTOFF		15 /**< Precedence cut off */
> +#define ODPH_NR_ICMP_UNREACH		15 /**< instead of hardcoding
> +						immediate value */
> 
>  /** Codes for REDIRECT. */
> -#define ICMP_REDIR_NET		0	/**< Redirect Net		*/
> -#define ICMP_REDIR_HOST		1	/**< Redirect Host		*/
> -#define ICMP_REDIR_NETTOS	2	/**< Redirect Net for TOS	*/
> -#define ICMP_REDIR_HOSTTOS	3	/**< Redirect Host for TOS	*/
> +#define ODPH_ICMP_REDIR_NET		0  /**< Redirect Net */
> +#define ODPH_ICMP_REDIR_HOST		1  /**< Redirect Host */
> +#define ODPH_ICMP_REDIR_NETTOS		2  /**< Redirect Net for TOS */
> +#define ODPH_ICMP_REDIR_HOSTTOS		3  /**< Redirect Host for TOS */
> 
>  /** Codes for TIME_EXCEEDED. */
> -#define ICMP_EXC_TTL		0	/**< TTL count exceeded		*/
> -#define ICMP_EXC_FRAGTIME	1	/**< Fragment Reass time
> -								exceeded*/
> +#define ODPH_ICMP_EXC_TTL		0  /**< TTL count exceeded */
> +#define ODPH_ICMP_EXC_FRAGTIME		1  /**< Fragment Reass time
> exceeded*/
> 
>  /** @internal Compile time assert */
>  ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
> "ODPH_ICMPHDR_T__SIZE_ERROR");
> diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> index ca71c44..4fb9c12 100644
> --- a/helper/include/odph_ip.h
> +++ b/helper/include/odph_ip.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * ODP IP header
> + * ODPH IP header
>   */
> 
>  #ifndef ODPH_IP_H_
> diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
> index 1ea349a..be91dd3 100644
> --- a/helper/include/odph_linux.h
> +++ b/helper/include/odph_linux.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * ODP Linux helper API
> + * ODPH Linux helper API
>   *
>   * This file is an optional helper to odp.h APIs. Application can manage
>   * pthreads also by other means.
> diff --git a/helper/include/odph_packet_helper.h
> b/helper/include/odph_packet_helper.h
> index c18f48d..c5315a9 100644
> --- a/helper/include/odph_packet_helper.h
> +++ b/helper/include/odph_packet_helper.h
> @@ -8,7 +8,7 @@
>  /**
>   * @file
>   *
> - * Optional ODP packet helper functions
> + * Optional ODPH packet helper functions
>   */
> 
>  #ifndef ODPH_PACKET_HELPER_H_
> diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
> index 76c1db8..82d2d13 100644
> --- a/helper/include/odph_ring.h
> +++ b/helper/include/odph_ring.h
> @@ -192,7 +192,7 @@ typedef struct odph_ring {
>   *    - ENOMEM - no appropriate memory area found in which to create
> memzone
>   */
>  odph_ring_t *odph_ring_create(const char *name, unsigned count,
> -			    unsigned flags);
> +			      unsigned flags);
> 
> 
>  /**
> --
> 1.9.1
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Maxim Uvarov Aug. 29, 2014, 12:19 p.m. UTC | #5

On 08/29/2014 02:53 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
> Hi,
>
> ODP/ODPH prefixes are needed always. First to make it clear what comes ODP/ODP helper, and secondly to avoid name space collisions (e.g. with linux/posix/application files that define similar things).
>
> -Petri
>
So that if somebody takes or app and wants to use generic headers 
instead of helper, then he needs to rename defines back?


>> -----Original Message-----
>> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
>> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
>> Sent: Friday, August 29, 2014 1:36 PM
>> To: lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
>>
>> I think we do not need to rename defines.  For example
>>
>> #define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
>>
>> is always ICMP_ECHOREPLY and is always 0. I suppose that defines were
>> copied from
>> some header file. And my opinion that we should have them as is, not
>> implementing new
>> names.
>>
>> For helper functions odph_ prefix is ok. But for standard defines it
>> look a little ugly.
>>
>> Best regards,
>> Maxim.
>>
>> On 08/29/2014 02:02 AM, Anders Roxell wrote:
>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>> ---
>>>    example/generator/odp_generator.c   |  6 +--
>>>    helper/include/odph_chksum.h        |  2 +-
>>>    helper/include/odph_eth.h           |  2 +-
>>>    helper/include/odph_icmp.h          | 79 ++++++++++++++++++-----------
>> --------
>>>    helper/include/odph_ip.h            |  2 +-
>>>    helper/include/odph_linux.h         |  2 +-
>>>    helper/include/odph_packet_helper.h |  2 +-
>>>    helper/include/odph_ring.h          |  2 +-
>>>    8 files changed, 48 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/example/generator/odp_generator.c
>> b/example/generator/odp_generator.c
>>> index 70c0353..246bccf 100644
>>> --- a/example/generator/odp_generator.c
>>> +++ b/example/generator/odp_generator.c
>>> @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>>>    	odph_ipv4_csum_update(pkt);
>>>    	/* icmp */
>>>    	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
>>> -	icmp->type = ICMP_ECHO;
>>> +	icmp->type = ODPH_ICMP_ECHO;
>>>    	icmp->code = 0;
>>>    	icmp->un.echo.id = 0;
>>>    	icmp->un.echo.sequence = ip->id;
>>> @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
>> pkt_tbl[], unsigned len)
>>>    		if (ip->proto == ODPH_IPPROTO_ICMP) {
>>>    			icmp = (odph_icmphdr_t *)(buf + offset);
>>>    			/* echo reply */
>>> -			if (icmp->type == ICMP_ECHOREPLY) {
>>> +			if (icmp->type == ODPH_ICMP_ECHOREPLY) {
>>>    				odp_atomic_inc_u64(&counters.icmp);
>>>    				memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
>>>    				       sizeof(struct timeval));
>>> @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
>> pkt_tbl[], unsigned len)
>>>    					"ICMP Echo Reply seq %d time %.1f ",
>>>    					odp_be_to_cpu_16(icmp->un.echo.sequence)
>>>    					, rtt);
>>> -			} else if (icmp->type == ICMP_ECHO) {
>>> +			} else if (icmp->type == ODPH_ICMP_ECHO) {
>>>    				rlen += sprintf(msg + rlen,
>>>    						"Icmp Echo Request");
>>>    			}
>>> diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
>>> index 710711a..3bcc2b6 100644
>>> --- a/helper/include/odph_chksum.h
>>> +++ b/helper/include/odph_chksum.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * ODP Checksum
>>> + * ODPH Checksum
>>>     */
>>>    #ifndef ODP_CHKSUM_H_
>>>    #define ODP_CHKSUM_H_
>>> diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
>>> index 55a2b1e..45f7fea 100644
>>> --- a/helper/include/odph_eth.h
>>> +++ b/helper/include/odph_eth.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * ODP ethernet header
>>> + * ODPH ethernet header
>>>     */
>>>
>>>    #ifndef ODPH_ETH_H_
>>> diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
>>> index 8414d7e..622f48b 100644
>>> --- a/helper/include/odph_icmp.h
>>> +++ b/helper/include/odph_icmp.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * ODP ICMP header
>>> + * ODPH ICMP header
>>>     */
>>>
>>>    #ifndef ODPH_ICMP_H_
>>> @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
>>>    	} un;			/**< icmp sub header */
>>>    } odph_icmphdr_t;
>>>
>>> -#define ICMP_ECHOREPLY		0	/**< Echo Reply			*/
>>> -#define ICMP_DEST_UNREACH	3	/**< Destination Unreachable	*/
>>> -#define ICMP_SOURCE_QUENCH	4	/**< Source Quench		*/
>>> -#define ICMP_REDIRECT		5	/**< Redirect (change route)	*/
>>> -#define ICMP_ECHO		8	/**< Echo Request		*/
>>> -#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
>>> -#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
>>> -#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
>>> -#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
>>> -#define ICMP_INFO_REQUEST	15	/**< Information Request	*/
>>> -#define ICMP_INFO_REPLY		16	/**< Information Reply		*/
>>> -#define ICMP_ADDRESS		17	/**< Address Mask Request	*/
>>> -#define ICMP_ADDRESSREPLY	18	/**< Address Mask Reply		*/
>>> -#define NR_ICMP_TYPES		18	/**< Number of icmp types	*/
>>> +#define ODPH_ICMP_ECHOREPLY		0  /**< Echo Reply */
>>> +#define ODPH_ICMP_DEST_UNREACH		3  /**< Destination Unreachable
>> */
>>> +#define ODPH_ICMP_SOURCE_QUENCH		4  /**< Source Quench */
>>> +#define ODPH_ICMP_REDIRECT		5  /**< Redirect (change route) */
>>> +#define ODPH_ICMP_ECHO			8  /**< Echo Request */
>>> +#define ODPH_ICMP_TIME_EXCEEDED		11 /**< Time Exceeded */
>>> +#define ODPH_ICMP_PARAMETERPROB		12 /**< Parameter Problem */
>>> +#define ODPH_ICMP_TIMESTAMP		13 /**< Timestamp Request */
>>> +#define ODPH_ICMP_TIMESTAMPREPLY	14 /**< Timestamp Reply */
>>> +#define ODPH_ICMP_INFO_REQUEST		15 /**< Information Request */
>>> +#define ODPH_ICMP_INFO_REPLY		16 /**< Information Reply */
>>> +#define ODPH_ICMP_ADDRESS		17 /**< Address Mask Request */
>>> +#define ODPH_ICMP_ADDRESSREPLY		18 /**< Address Mask Reply */
>>> +#define ODPH_NR_ICMP_TYPES		18 /**< Number of icmp types */
>>>
>>>    /** Codes for UNREACH. */
>>> -#define ICMP_NET_UNREACH	0	/**< Network Unreachable	*/
>>> -#define ICMP_HOST_UNREACH	1	/**< Host Unreachable		*/
>>> -#define ICMP_PROT_UNREACH	2	/**< Protocol Unreachable	*/
>>> -#define ICMP_PORT_UNREACH	3	/**< Port Unreachable		*/
>>> -#define ICMP_FRAG_NEEDED	4	/**< Fragmentation Needed/DF set*/
>>> -#define ICMP_SR_FAILED		5	/**< Source Route failed	*/
>>> -#define ICMP_NET_UNKNOWN	6	/**< Network Unknown		*/
>>> -#define ICMP_HOST_UNKNOWN	7	/**< Host Unknown		*/
>>> -#define ICMP_HOST_ISOLATED	8	/**< Host Isolated		*/
>>> -#define ICMP_NET_ANO		9	/**< ICMP_NET_ANO		*/
>>> -#define ICMP_HOST_ANO		10	/**< ICMP_HOST_ANO		*/
>>> -#define ICMP_NET_UNR_TOS	11	/**< ICMP_NET_UNR_TOS		*/
>>> -#define ICMP_HOST_UNR_TOS	12	/**< ICMP_HOST_UNR_TOS		*/
>>> -#define ICMP_PKT_FILTERED	13	/**< Packet filtered		*/
>>> -#define ICMP_PREC_VIOLATION	14	/**< Precedence violation	*/
>>> -#define ICMP_PREC_CUTOFF	15	/**< Precedence cut off		*/
>>> -#define NR_ICMP_UNREACH		15	/**< instead of hardcoding
>>> -							immediate value */
>>> +#define ODPH_ICMP_NET_UNREACH		0  /**< Network Unreachable */
>>> +#define ODPH_ICMP_HOST_UNREACH		1  /**< Host Unreachable */
>>> +#define ODPH_ICMP_PROT_UNREACH		2  /**< Protocol Unreachable */
>>> +#define ODPH_ICMP_PORT_UNREACH		3  /**< Port Unreachable */
>>> +#define ODPH_ICMP_FRAG_NEEDED		4  /**< Fragmentation Needed/DF
>> set */
>>> +#define ODPH_ICMP_SR_FAILED		5  /**< Source Route failed */
>>> +#define ODPH_ICMP_NET_UNKNOWN		6  /**< Network Unknown */
>>> +#define ODPH_ICMP_HOST_UNKNOWN		7  /**< Host Unknown */
>>> +#define ODPH_ICMP_HOST_ISOLATED		8  /**< Host Isolated */
>>> +#define ODPH_ICMP_NET_ANO		9  /**< ICMP_NET_ANO */
>>> +#define ODPH_ICMP_HOST_ANO		10 /**< ICMP_HOST_ANO */
>>> +#define ODPH_ICMP_NET_UNR_TOS		11 /**< ICMP_NET_UNR_TOS */
>>> +#define ODPH_ICMP_HOST_UNR_TOS		12 /**< ICMP_HOST_UNR_TOS */
>>> +#define ODPH_ICMP_PKT_FILTERED		13 /**< Packet filtered */
>>> +#define ODPH_ICMP_PREC_VIOLATION	14 /**< Precedence violation */
>>> +#define ODPH_ICMP_PREC_CUTOFF		15 /**< Precedence cut off */
>>> +#define ODPH_NR_ICMP_UNREACH		15 /**< instead of hardcoding
>>> +						immediate value */
>>>
>>>    /** Codes for REDIRECT. */
>>> -#define ICMP_REDIR_NET		0	/**< Redirect Net		*/
>>> -#define ICMP_REDIR_HOST		1	/**< Redirect Host		*/
>>> -#define ICMP_REDIR_NETTOS	2	/**< Redirect Net for TOS	*/
>>> -#define ICMP_REDIR_HOSTTOS	3	/**< Redirect Host for TOS	*/
>>> +#define ODPH_ICMP_REDIR_NET		0  /**< Redirect Net */
>>> +#define ODPH_ICMP_REDIR_HOST		1  /**< Redirect Host */
>>> +#define ODPH_ICMP_REDIR_NETTOS		2  /**< Redirect Net for TOS */
>>> +#define ODPH_ICMP_REDIR_HOSTTOS		3  /**< Redirect Host for TOS */
>>>
>>>    /** Codes for TIME_EXCEEDED. */
>>> -#define ICMP_EXC_TTL		0	/**< TTL count exceeded		*/
>>> -#define ICMP_EXC_FRAGTIME	1	/**< Fragment Reass time
>>> -								exceeded*/
>>> +#define ODPH_ICMP_EXC_TTL		0  /**< TTL count exceeded */
>>> +#define ODPH_ICMP_EXC_FRAGTIME		1  /**< Fragment Reass time
>> exceeded*/
>>>    /** @internal Compile time assert */
>>>    ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
>> "ODPH_ICMPHDR_T__SIZE_ERROR");
>>> diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
>>> index ca71c44..4fb9c12 100644
>>> --- a/helper/include/odph_ip.h
>>> +++ b/helper/include/odph_ip.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * ODP IP header
>>> + * ODPH IP header
>>>     */
>>>
>>>    #ifndef ODPH_IP_H_
>>> diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
>>> index 1ea349a..be91dd3 100644
>>> --- a/helper/include/odph_linux.h
>>> +++ b/helper/include/odph_linux.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * ODP Linux helper API
>>> + * ODPH Linux helper API
>>>     *
>>>     * This file is an optional helper to odp.h APIs. Application can
>> manage
>>>     * pthreads also by other means.
>>> diff --git a/helper/include/odph_packet_helper.h
>> b/helper/include/odph_packet_helper.h
>>> index c18f48d..c5315a9 100644
>>> --- a/helper/include/odph_packet_helper.h
>>> +++ b/helper/include/odph_packet_helper.h
>>> @@ -8,7 +8,7 @@
>>>    /**
>>>     * @file
>>>     *
>>> - * Optional ODP packet helper functions
>>> + * Optional ODPH packet helper functions
>>>     */
>>>
>>>    #ifndef ODPH_PACKET_HELPER_H_
>>> diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
>>> index 76c1db8..82d2d13 100644
>>> --- a/helper/include/odph_ring.h
>>> +++ b/helper/include/odph_ring.h
>>> @@ -192,7 +192,7 @@ typedef struct odph_ring {
>>>     *    - ENOMEM - no appropriate memory area found in which to create
>> memzone
>>>     */
>>>    odph_ring_t *odph_ring_create(const char *name, unsigned count,
>>> -			    unsigned flags);
>>> +			      unsigned flags);
>>>
>>>
>>>    /**
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp

Bill Fischofer Aug. 29, 2014, 12:21 p.m. UTC | #6

I agree with Petri. Since C has a single global namespace for #defines the
real issue is name collisions with applications using ODP.

Bill


On Fri, Aug 29, 2014 at 6:49 AM, Savolainen, Petri (NSN - FI/Espoo) <
petri.savolainen@nsn.com> wrote:

>
>
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of ext Anders Roxell
> > Sent: Friday, August 29, 2014 1:02 AM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> >
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> Acked-by: Petri Savolainen <petri.savolainen@linaro.org>
> > ---
> >  example/generator/odp_generator.c   |  6 +--
> >  helper/include/odph_chksum.h        |  2 +-
> >  helper/include/odph_eth.h           |  2 +-
> >  helper/include/odph_icmp.h          | 79
> ++++++++++++++++++--------------
> > -----
> >  helper/include/odph_ip.h            |  2 +-
> >  helper/include/odph_linux.h         |  2 +-
> >  helper/include/odph_packet_helper.h |  2 +-
> >  helper/include/odph_ring.h          |  2 +-
> >  8 files changed, 48 insertions(+), 49 deletions(-)
> >
> > diff --git a/example/generator/odp_generator.c
> > b/example/generator/odp_generator.c
> > index 70c0353..246bccf 100644
> > --- a/example/generator/odp_generator.c
> > +++ b/example/generator/odp_generator.c
> > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
> >       odph_ipv4_csum_update(pkt);
> >       /* icmp */
> >       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +
> ODPH_IPV4HDR_LEN);
> > -     icmp->type = ICMP_ECHO;
> > +     icmp->type = ODPH_ICMP_ECHO;
> >       icmp->code = 0;
> >       icmp->un.echo.id = 0;
> >       icmp->un.echo.sequence = ip->id;
> > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
> > pkt_tbl[], unsigned len)
> >               if (ip->proto == ODPH_IPPROTO_ICMP) {
> >                       icmp = (odph_icmphdr_t *)(buf + offset);
> >                       /* echo reply */
> > -                     if (icmp->type == ICMP_ECHOREPLY) {
> > +                     if (icmp->type == ODPH_ICMP_ECHOREPLY) {
> >                               odp_atomic_inc_u64(&counters.icmp);
> >                               memcpy(&tvsend, buf + offset +
> ODPH_ICMPHDR_LEN,
> >                                      sizeof(struct timeval));
> > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
> > pkt_tbl[], unsigned len)
> >                                       "ICMP Echo Reply seq %d time %.1f
> ",
> >
>  odp_be_to_cpu_16(icmp->un.echo.sequence)
> >                                       , rtt);
> > -                     } else if (icmp->type == ICMP_ECHO) {
> > +                     } else if (icmp->type == ODPH_ICMP_ECHO) {
> >                               rlen += sprintf(msg + rlen,
> >                                               "Icmp Echo Request");
> >                       }
> > diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
> > index 710711a..3bcc2b6 100644
> > --- a/helper/include/odph_chksum.h
> > +++ b/helper/include/odph_chksum.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * ODP Checksum
> > + * ODPH Checksum
> >   */
> >  #ifndef ODP_CHKSUM_H_
> >  #define ODP_CHKSUM_H_
> > diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
> > index 55a2b1e..45f7fea 100644
> > --- a/helper/include/odph_eth.h
> > +++ b/helper/include/odph_eth.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * ODP ethernet header
> > + * ODPH ethernet header
> >   */
> >
> >  #ifndef ODPH_ETH_H_
> > diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
> > index 8414d7e..622f48b 100644
> > --- a/helper/include/odph_icmp.h
> > +++ b/helper/include/odph_icmp.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * ODP ICMP header
> > + * ODPH ICMP header
> >   */
> >
> >  #ifndef ODPH_ICMP_H_
> > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
> >       } un;                   /**< icmp sub header */
> >  } odph_icmphdr_t;
> >
> > -#define ICMP_ECHOREPLY               0       /**< Echo Reply
>      */
> > -#define ICMP_DEST_UNREACH    3       /**< Destination Unreachable    */
> > -#define ICMP_SOURCE_QUENCH   4       /**< Source Quench              */
> > -#define ICMP_REDIRECT                5       /**< Redirect (change
> route)    */
> > -#define ICMP_ECHO            8       /**< Echo Request               */
> > -#define ICMP_TIME_EXCEEDED   11      /**< Time Exceeded              */
> > -#define ICMP_PARAMETERPROB   12      /**< Parameter Problem          */
> > -#define ICMP_TIMESTAMP               13      /**< Timestamp Request
>       */
> > -#define ICMP_TIMESTAMPREPLY  14      /**< Timestamp Reply            */
> > -#define ICMP_INFO_REQUEST    15      /**< Information Request        */
> > -#define ICMP_INFO_REPLY              16      /**< Information Reply
>       */
> > -#define ICMP_ADDRESS         17      /**< Address Mask Request       */
> > -#define ICMP_ADDRESSREPLY    18      /**< Address Mask Reply         */
> > -#define NR_ICMP_TYPES                18      /**< Number of icmp types
>      */
> > +#define ODPH_ICMP_ECHOREPLY          0  /**< Echo Reply */
> > +#define ODPH_ICMP_DEST_UNREACH               3  /**< Destination
> Unreachable
> > */
> > +#define ODPH_ICMP_SOURCE_QUENCH              4  /**< Source Quench */
> > +#define ODPH_ICMP_REDIRECT           5  /**< Redirect (change route) */
> > +#define ODPH_ICMP_ECHO                       8  /**< Echo Request */
> > +#define ODPH_ICMP_TIME_EXCEEDED              11 /**< Time Exceeded */
> > +#define ODPH_ICMP_PARAMETERPROB              12 /**< Parameter Problem
> */
> > +#define ODPH_ICMP_TIMESTAMP          13 /**< Timestamp Request */
> > +#define ODPH_ICMP_TIMESTAMPREPLY     14 /**< Timestamp Reply */
> > +#define ODPH_ICMP_INFO_REQUEST               15 /**< Information
> Request */
> > +#define ODPH_ICMP_INFO_REPLY         16 /**< Information Reply */
> > +#define ODPH_ICMP_ADDRESS            17 /**< Address Mask Request */
> > +#define ODPH_ICMP_ADDRESSREPLY               18 /**< Address Mask Reply
> */
> > +#define ODPH_NR_ICMP_TYPES           18 /**< Number of icmp types */
> >
> >  /** Codes for UNREACH. */
> > -#define ICMP_NET_UNREACH     0       /**< Network Unreachable        */
> > -#define ICMP_HOST_UNREACH    1       /**< Host Unreachable           */
> > -#define ICMP_PROT_UNREACH    2       /**< Protocol Unreachable       */
> > -#define ICMP_PORT_UNREACH    3       /**< Port Unreachable           */
> > -#define ICMP_FRAG_NEEDED     4       /**< Fragmentation Needed/DF set*/
> > -#define ICMP_SR_FAILED               5       /**< Source Route failed
>       */
> > -#define ICMP_NET_UNKNOWN     6       /**< Network Unknown            */
> > -#define ICMP_HOST_UNKNOWN    7       /**< Host Unknown               */
> > -#define ICMP_HOST_ISOLATED   8       /**< Host Isolated              */
> > -#define ICMP_NET_ANO         9       /**< ICMP_NET_ANO               */
> > -#define ICMP_HOST_ANO                10      /**< ICMP_HOST_ANO
>       */
> > -#define ICMP_NET_UNR_TOS     11      /**< ICMP_NET_UNR_TOS           */
> > -#define ICMP_HOST_UNR_TOS    12      /**< ICMP_HOST_UNR_TOS          */
> > -#define ICMP_PKT_FILTERED    13      /**< Packet filtered            */
> > -#define ICMP_PREC_VIOLATION  14      /**< Precedence violation       */
> > -#define ICMP_PREC_CUTOFF     15      /**< Precedence cut off         */
> > -#define NR_ICMP_UNREACH              15      /**< instead of hardcoding
> > -                                                     immediate value */
> > +#define ODPH_ICMP_NET_UNREACH                0  /**< Network
> Unreachable */
> > +#define ODPH_ICMP_HOST_UNREACH               1  /**< Host Unreachable */
> > +#define ODPH_ICMP_PROT_UNREACH               2  /**< Protocol
> Unreachable */
> > +#define ODPH_ICMP_PORT_UNREACH               3  /**< Port Unreachable */
> > +#define ODPH_ICMP_FRAG_NEEDED                4  /**< Fragmentation
> Needed/DF
> > set */
> > +#define ODPH_ICMP_SR_FAILED          5  /**< Source Route failed */
> > +#define ODPH_ICMP_NET_UNKNOWN                6  /**< Network Unknown */
> > +#define ODPH_ICMP_HOST_UNKNOWN               7  /**< Host Unknown */
> > +#define ODPH_ICMP_HOST_ISOLATED              8  /**< Host Isolated */
> > +#define ODPH_ICMP_NET_ANO            9  /**< ICMP_NET_ANO */
> > +#define ODPH_ICMP_HOST_ANO           10 /**< ICMP_HOST_ANO */
> > +#define ODPH_ICMP_NET_UNR_TOS                11 /**< ICMP_NET_UNR_TOS */
> > +#define ODPH_ICMP_HOST_UNR_TOS               12 /**< ICMP_HOST_UNR_TOS
> */
> > +#define ODPH_ICMP_PKT_FILTERED               13 /**< Packet filtered */
> > +#define ODPH_ICMP_PREC_VIOLATION     14 /**< Precedence violation */
> > +#define ODPH_ICMP_PREC_CUTOFF                15 /**< Precedence cut off
> */
> > +#define ODPH_NR_ICMP_UNREACH         15 /**< instead of hardcoding
> > +                                             immediate value */
> >
> >  /** Codes for REDIRECT. */
> > -#define ICMP_REDIR_NET               0       /**< Redirect Net
>      */
> > -#define ICMP_REDIR_HOST              1       /**< Redirect Host
>       */
> > -#define ICMP_REDIR_NETTOS    2       /**< Redirect Net for TOS       */
> > -#define ICMP_REDIR_HOSTTOS   3       /**< Redirect Host for TOS      */
> > +#define ODPH_ICMP_REDIR_NET          0  /**< Redirect Net */
> > +#define ODPH_ICMP_REDIR_HOST         1  /**< Redirect Host */
> > +#define ODPH_ICMP_REDIR_NETTOS               2  /**< Redirect Net for
> TOS */
> > +#define ODPH_ICMP_REDIR_HOSTTOS              3  /**< Redirect Host for
> TOS */
> >
> >  /** Codes for TIME_EXCEEDED. */
> > -#define ICMP_EXC_TTL         0       /**< TTL count exceeded         */
> > -#define ICMP_EXC_FRAGTIME    1       /**< Fragment Reass time
> > -                                                             exceeded*/
> > +#define ODPH_ICMP_EXC_TTL            0  /**< TTL count exceeded */
> > +#define ODPH_ICMP_EXC_FRAGTIME               1  /**< Fragment Reass time
> > exceeded*/
> >
> >  /** @internal Compile time assert */
> >  ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
> > "ODPH_ICMPHDR_T__SIZE_ERROR");
> > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> > index ca71c44..4fb9c12 100644
> > --- a/helper/include/odph_ip.h
> > +++ b/helper/include/odph_ip.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * ODP IP header
> > + * ODPH IP header
> >   */
> >
> >  #ifndef ODPH_IP_H_
> > diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
> > index 1ea349a..be91dd3 100644
> > --- a/helper/include/odph_linux.h
> > +++ b/helper/include/odph_linux.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * ODP Linux helper API
> > + * ODPH Linux helper API
> >   *
> >   * This file is an optional helper to odp.h APIs. Application can manage
> >   * pthreads also by other means.
> > diff --git a/helper/include/odph_packet_helper.h
> > b/helper/include/odph_packet_helper.h
> > index c18f48d..c5315a9 100644
> > --- a/helper/include/odph_packet_helper.h
> > +++ b/helper/include/odph_packet_helper.h
> > @@ -8,7 +8,7 @@
> >  /**
> >   * @file
> >   *
> > - * Optional ODP packet helper functions
> > + * Optional ODPH packet helper functions
> >   */
> >
> >  #ifndef ODPH_PACKET_HELPER_H_
> > diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
> > index 76c1db8..82d2d13 100644
> > --- a/helper/include/odph_ring.h
> > +++ b/helper/include/odph_ring.h
> > @@ -192,7 +192,7 @@ typedef struct odph_ring {
> >   *    - ENOMEM - no appropriate memory area found in which to create
> > memzone
> >   */
> >  odph_ring_t *odph_ring_create(const char *name, unsigned count,
> > -                         unsigned flags);
> > +                           unsigned flags);
> >
> >
> >  /**
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

Mike Holmes Aug. 29, 2014, 6:10 p.m. UTC | #7

Maxim,

Does this impact interworking with other packages like SNORT etc in your
experience ?
Is it likely that someone will need to convert to generic headers, do you
have a use case that this might be an issue?

I think we need to scope the defines, but would like to be sure we are not
making a lot of peoples lives difficult.

Mike


On 29 August 2014 08:21, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> I agree with Petri. Since C has a single global namespace for #defines the
> real issue is name collisions with applications using ODP.
>
> Bill
>
>
> On Fri, Aug 29, 2014 at 6:49 AM, Savolainen, Petri (NSN - FI/Espoo) <
> petri.savolainen@nsn.com> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
>> > bounces@lists.linaro.org] On Behalf Of ext Anders Roxell
>> > Sent: Friday, August 29, 2014 1:02 AM
>> > To: lng-odp@lists.linaro.org
>> > Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
>> >
>> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>> Acked-by: Petri Savolainen <petri.savolainen@linaro.org>
>> > ---
>> >  example/generator/odp_generator.c   |  6 +--
>> >  helper/include/odph_chksum.h        |  2 +-
>> >  helper/include/odph_eth.h           |  2 +-
>> >  helper/include/odph_icmp.h          | 79
>> ++++++++++++++++++--------------
>> > -----
>> >  helper/include/odph_ip.h            |  2 +-
>> >  helper/include/odph_linux.h         |  2 +-
>> >  helper/include/odph_packet_helper.h |  2 +-
>> >  helper/include/odph_ring.h          |  2 +-
>> >  8 files changed, 48 insertions(+), 49 deletions(-)
>> >
>> > diff --git a/example/generator/odp_generator.c
>> > b/example/generator/odp_generator.c
>> > index 70c0353..246bccf 100644
>> > --- a/example/generator/odp_generator.c
>> > +++ b/example/generator/odp_generator.c
>> > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>> >       odph_ipv4_csum_update(pkt);
>> >       /* icmp */
>> >       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +
>> ODPH_IPV4HDR_LEN);
>> > -     icmp->type = ICMP_ECHO;
>> > +     icmp->type = ODPH_ICMP_ECHO;
>> >       icmp->code = 0;
>> >       icmp->un.echo.id = 0;
>> >       icmp->un.echo.sequence = ip->id;
>> > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
>> > pkt_tbl[], unsigned len)
>> >               if (ip->proto == ODPH_IPPROTO_ICMP) {
>> >                       icmp = (odph_icmphdr_t *)(buf + offset);
>> >                       /* echo reply */
>> > -                     if (icmp->type == ICMP_ECHOREPLY) {
>> > +                     if (icmp->type == ODPH_ICMP_ECHOREPLY) {
>> >                               odp_atomic_inc_u64(&counters.icmp);
>> >                               memcpy(&tvsend, buf + offset +
>> ODPH_ICMPHDR_LEN,
>> >                                      sizeof(struct timeval));
>> > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
>> > pkt_tbl[], unsigned len)
>> >                                       "ICMP Echo Reply seq %d time %.1f
>> ",
>> >
>>  odp_be_to_cpu_16(icmp->un.echo.sequence)
>> >                                       , rtt);
>> > -                     } else if (icmp->type == ICMP_ECHO) {
>> > +                     } else if (icmp->type == ODPH_ICMP_ECHO) {
>> >                               rlen += sprintf(msg + rlen,
>> >                                               "Icmp Echo Request");
>> >                       }
>> > diff --git a/helper/include/odph_chksum.h b/helper/include/odph_chksum.h
>> > index 710711a..3bcc2b6 100644
>> > --- a/helper/include/odph_chksum.h
>> > +++ b/helper/include/odph_chksum.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * ODP Checksum
>> > + * ODPH Checksum
>> >   */
>> >  #ifndef ODP_CHKSUM_H_
>> >  #define ODP_CHKSUM_H_
>> > diff --git a/helper/include/odph_eth.h b/helper/include/odph_eth.h
>> > index 55a2b1e..45f7fea 100644
>> > --- a/helper/include/odph_eth.h
>> > +++ b/helper/include/odph_eth.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * ODP ethernet header
>> > + * ODPH ethernet header
>> >   */
>> >
>> >  #ifndef ODPH_ETH_H_
>> > diff --git a/helper/include/odph_icmp.h b/helper/include/odph_icmp.h
>> > index 8414d7e..622f48b 100644
>> > --- a/helper/include/odph_icmp.h
>> > +++ b/helper/include/odph_icmp.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * ODP ICMP header
>> > + * ODPH ICMP header
>> >   */
>> >
>> >  #ifndef ODPH_ICMP_H_
>> > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
>> >       } un;                   /**< icmp sub header */
>> >  } odph_icmphdr_t;
>> >
>> > -#define ICMP_ECHOREPLY               0       /**< Echo Reply
>>        */
>> > -#define ICMP_DEST_UNREACH    3       /**< Destination Unreachable    */
>> > -#define ICMP_SOURCE_QUENCH   4       /**< Source Quench              */
>> > -#define ICMP_REDIRECT                5       /**< Redirect (change
>> route)    */
>> > -#define ICMP_ECHO            8       /**< Echo Request               */
>> > -#define ICMP_TIME_EXCEEDED   11      /**< Time Exceeded              */
>> > -#define ICMP_PARAMETERPROB   12      /**< Parameter Problem          */
>> > -#define ICMP_TIMESTAMP               13      /**< Timestamp Request
>>       */
>> > -#define ICMP_TIMESTAMPREPLY  14      /**< Timestamp Reply            */
>> > -#define ICMP_INFO_REQUEST    15      /**< Information Request        */
>> > -#define ICMP_INFO_REPLY              16      /**< Information Reply
>>       */
>> > -#define ICMP_ADDRESS         17      /**< Address Mask Request       */
>> > -#define ICMP_ADDRESSREPLY    18      /**< Address Mask Reply         */
>> > -#define NR_ICMP_TYPES                18      /**< Number of icmp
>> types       */
>> > +#define ODPH_ICMP_ECHOREPLY          0  /**< Echo Reply */
>> > +#define ODPH_ICMP_DEST_UNREACH               3  /**< Destination
>> Unreachable
>> > */
>> > +#define ODPH_ICMP_SOURCE_QUENCH              4  /**< Source Quench */
>> > +#define ODPH_ICMP_REDIRECT           5  /**< Redirect (change route) */
>> > +#define ODPH_ICMP_ECHO                       8  /**< Echo Request */
>> > +#define ODPH_ICMP_TIME_EXCEEDED              11 /**< Time Exceeded */
>> > +#define ODPH_ICMP_PARAMETERPROB              12 /**< Parameter Problem
>> */
>> > +#define ODPH_ICMP_TIMESTAMP          13 /**< Timestamp Request */
>> > +#define ODPH_ICMP_TIMESTAMPREPLY     14 /**< Timestamp Reply */
>> > +#define ODPH_ICMP_INFO_REQUEST               15 /**< Information
>> Request */
>> > +#define ODPH_ICMP_INFO_REPLY         16 /**< Information Reply */
>> > +#define ODPH_ICMP_ADDRESS            17 /**< Address Mask Request */
>> > +#define ODPH_ICMP_ADDRESSREPLY               18 /**< Address Mask
>> Reply */
>> > +#define ODPH_NR_ICMP_TYPES           18 /**< Number of icmp types */
>> >
>> >  /** Codes for UNREACH. */
>> > -#define ICMP_NET_UNREACH     0       /**< Network Unreachable        */
>> > -#define ICMP_HOST_UNREACH    1       /**< Host Unreachable           */
>> > -#define ICMP_PROT_UNREACH    2       /**< Protocol Unreachable       */
>> > -#define ICMP_PORT_UNREACH    3       /**< Port Unreachable           */
>> > -#define ICMP_FRAG_NEEDED     4       /**< Fragmentation Needed/DF set*/
>> > -#define ICMP_SR_FAILED               5       /**< Source Route failed
>>       */
>> > -#define ICMP_NET_UNKNOWN     6       /**< Network Unknown            */
>> > -#define ICMP_HOST_UNKNOWN    7       /**< Host Unknown               */
>> > -#define ICMP_HOST_ISOLATED   8       /**< Host Isolated              */
>> > -#define ICMP_NET_ANO         9       /**< ICMP_NET_ANO               */
>> > -#define ICMP_HOST_ANO                10      /**< ICMP_HOST_ANO
>>       */
>> > -#define ICMP_NET_UNR_TOS     11      /**< ICMP_NET_UNR_TOS           */
>> > -#define ICMP_HOST_UNR_TOS    12      /**< ICMP_HOST_UNR_TOS          */
>> > -#define ICMP_PKT_FILTERED    13      /**< Packet filtered            */
>> > -#define ICMP_PREC_VIOLATION  14      /**< Precedence violation       */
>> > -#define ICMP_PREC_CUTOFF     15      /**< Precedence cut off         */
>> > -#define NR_ICMP_UNREACH              15      /**< instead of hardcoding
>> > -                                                     immediate value */
>> > +#define ODPH_ICMP_NET_UNREACH                0  /**< Network
>> Unreachable */
>> > +#define ODPH_ICMP_HOST_UNREACH               1  /**< Host Unreachable
>> */
>> > +#define ODPH_ICMP_PROT_UNREACH               2  /**< Protocol
>> Unreachable */
>> > +#define ODPH_ICMP_PORT_UNREACH               3  /**< Port Unreachable
>> */
>> > +#define ODPH_ICMP_FRAG_NEEDED                4  /**< Fragmentation
>> Needed/DF
>> > set */
>> > +#define ODPH_ICMP_SR_FAILED          5  /**< Source Route failed */
>> > +#define ODPH_ICMP_NET_UNKNOWN                6  /**< Network Unknown */
>> > +#define ODPH_ICMP_HOST_UNKNOWN               7  /**< Host Unknown */
>> > +#define ODPH_ICMP_HOST_ISOLATED              8  /**< Host Isolated */
>> > +#define ODPH_ICMP_NET_ANO            9  /**< ICMP_NET_ANO */
>> > +#define ODPH_ICMP_HOST_ANO           10 /**< ICMP_HOST_ANO */
>> > +#define ODPH_ICMP_NET_UNR_TOS                11 /**< ICMP_NET_UNR_TOS
>> */
>> > +#define ODPH_ICMP_HOST_UNR_TOS               12 /**< ICMP_HOST_UNR_TOS
>> */
>> > +#define ODPH_ICMP_PKT_FILTERED               13 /**< Packet filtered */
>> > +#define ODPH_ICMP_PREC_VIOLATION     14 /**< Precedence violation */
>> > +#define ODPH_ICMP_PREC_CUTOFF                15 /**< Precedence cut
>> off */
>> > +#define ODPH_NR_ICMP_UNREACH         15 /**< instead of hardcoding
>> > +                                             immediate value */
>> >
>> >  /** Codes for REDIRECT. */
>> > -#define ICMP_REDIR_NET               0       /**< Redirect Net
>>        */
>> > -#define ICMP_REDIR_HOST              1       /**< Redirect Host
>>       */
>> > -#define ICMP_REDIR_NETTOS    2       /**< Redirect Net for TOS       */
>> > -#define ICMP_REDIR_HOSTTOS   3       /**< Redirect Host for TOS      */
>> > +#define ODPH_ICMP_REDIR_NET          0  /**< Redirect Net */
>> > +#define ODPH_ICMP_REDIR_HOST         1  /**< Redirect Host */
>> > +#define ODPH_ICMP_REDIR_NETTOS               2  /**< Redirect Net for
>> TOS */
>> > +#define ODPH_ICMP_REDIR_HOSTTOS              3  /**< Redirect Host for
>> TOS */
>> >
>> >  /** Codes for TIME_EXCEEDED. */
>> > -#define ICMP_EXC_TTL         0       /**< TTL count exceeded         */
>> > -#define ICMP_EXC_FRAGTIME    1       /**< Fragment Reass time
>> > -                                                             exceeded*/
>> > +#define ODPH_ICMP_EXC_TTL            0  /**< TTL count exceeded */
>> > +#define ODPH_ICMP_EXC_FRAGTIME               1  /**< Fragment Reass
>> time
>> > exceeded*/
>> >
>> >  /** @internal Compile time assert */
>> >  ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
>> > "ODPH_ICMPHDR_T__SIZE_ERROR");
>> > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
>> > index ca71c44..4fb9c12 100644
>> > --- a/helper/include/odph_ip.h
>> > +++ b/helper/include/odph_ip.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * ODP IP header
>> > + * ODPH IP header
>> >   */
>> >
>> >  #ifndef ODPH_IP_H_
>> > diff --git a/helper/include/odph_linux.h b/helper/include/odph_linux.h
>> > index 1ea349a..be91dd3 100644
>> > --- a/helper/include/odph_linux.h
>> > +++ b/helper/include/odph_linux.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * ODP Linux helper API
>> > + * ODPH Linux helper API
>> >   *
>> >   * This file is an optional helper to odp.h APIs. Application can
>> manage
>> >   * pthreads also by other means.
>> > diff --git a/helper/include/odph_packet_helper.h
>> > b/helper/include/odph_packet_helper.h
>> > index c18f48d..c5315a9 100644
>> > --- a/helper/include/odph_packet_helper.h
>> > +++ b/helper/include/odph_packet_helper.h
>> > @@ -8,7 +8,7 @@
>> >  /**
>> >   * @file
>> >   *
>> > - * Optional ODP packet helper functions
>> > + * Optional ODPH packet helper functions
>> >   */
>> >
>> >  #ifndef ODPH_PACKET_HELPER_H_
>> > diff --git a/helper/include/odph_ring.h b/helper/include/odph_ring.h
>> > index 76c1db8..82d2d13 100644
>> > --- a/helper/include/odph_ring.h
>> > +++ b/helper/include/odph_ring.h
>> > @@ -192,7 +192,7 @@ typedef struct odph_ring {
>> >   *    - ENOMEM - no appropriate memory area found in which to create
>> > memzone
>> >   */
>> >  odph_ring_t *odph_ring_create(const char *name, unsigned count,
>> > -                         unsigned flags);
>> > +                           unsigned flags);
>> >
>> >
>> >  /**
>> > --
>> > 1.9.1
>> >
>> >
>> > _______________________________________________
>> > lng-odp mailing list
>> > lng-odp@lists.linaro.org
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Maxim Uvarov Sept. 1, 2014, 10:41 a.m. UTC | #8

My point is that defines are unrolled on preprocessor stage. While name 
spaces on link stage.
So that if you do not change define value you can have several defines 
for that in different files.
Gcc will not warn on that.  Only if the same define will be defined with 
different name.

Example:
odp.h:
#define AAA 1
#define AAA 1

Will not warn, and it's valid.

I do not see the reason why we have to change define for ICMP ECHO bit 
to some odp specific. While in fact we will not change it
in future.

Does it sound reasonable?

Best regards,
Maxim.

On 08/29/2014 10:10 PM, Mike Holmes wrote:
> Maxim,
>
> Does this impact interworking with other packages like SNORT etc in 
> your experience ?
> Is it likely that someone will need to convert to generic headers, do 
> you have a use case that this might be an issue?
>
> I think we need to scope the defines, but would like to be sure we are 
> not making a lot of peoples lives difficult.
>
> Mike
>
>
> On 29 August 2014 08:21, Bill Fischofer <bill.fischofer@linaro.org 
> <mailto:bill.fischofer@linaro.org>> wrote:
>
>     I agree with Petri. Since C has a single global namespace for
>     #defines the real issue is name collisions with applications using
>     ODP.
>
>     Bill
>
>
>     On Fri, Aug 29, 2014 at 6:49 AM, Savolainen, Petri (NSN -
>     FI/Espoo) <petri.savolainen@nsn.com
>     <mailto:petri.savolainen@nsn.com>> wrote:
>
>
>
>         > -----Original Message-----
>         > From: lng-odp-bounces@lists.linaro.org
>         <mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-
>         <mailto:lng-odp->
>         > bounces@lists.linaro.org <mailto:bounces@lists.linaro.org>]
>         On Behalf Of ext Anders Roxell
>         > Sent: Friday, August 29, 2014 1:02 AM
>         > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>         > Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
>         >
>         > Signed-off-by: Anders Roxell <anders.roxell@linaro.org
>         <mailto:anders.roxell@linaro.org>>
>         Acked-by: Petri Savolainen <petri.savolainen@linaro.org
>         <mailto:petri.savolainen@linaro.org>>
>         > ---
>         >  example/generator/odp_generator.c   |  6 +--
>         >  helper/include/odph_chksum.h        |  2 +-
>         >  helper/include/odph_eth.h           |  2 +-
>         >  helper/include/odph_icmp.h          | 79
>         ++++++++++++++++++--------------
>         > -----
>         >  helper/include/odph_ip.h            |  2 +-
>         >  helper/include/odph_linux.h         |  2 +-
>         >  helper/include/odph_packet_helper.h |  2 +-
>         >  helper/include/odph_ring.h          |  2 +-
>         >  8 files changed, 48 insertions(+), 49 deletions(-)
>         >
>         > diff --git a/example/generator/odp_generator.c
>         > b/example/generator/odp_generator.c
>         > index 70c0353..246bccf 100644
>         > --- a/example/generator/odp_generator.c
>         > +++ b/example/generator/odp_generator.c
>         > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>         >       odph_ipv4_csum_update(pkt);
>         >       /* icmp */
>         >       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +
>         ODPH_IPV4HDR_LEN);
>         > -     icmp->type = ICMP_ECHO;
>         > +     icmp->type = ODPH_ICMP_ECHO;
>         >       icmp->code = 0;
>         >       icmp->un.echo.id <http://un.echo.id> = 0;
>         >       icmp->un.echo.sequence = ip->id;
>         > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
>         > pkt_tbl[], unsigned len)
>         >               if (ip->proto == ODPH_IPPROTO_ICMP) {
>         >                       icmp = (odph_icmphdr_t *)(buf + offset);
>         >                       /* echo reply */
>         > -                     if (icmp->type == ICMP_ECHOREPLY) {
>         > +                     if (icmp->type == ODPH_ICMP_ECHOREPLY) {
>         >  odp_atomic_inc_u64(&counters.icmp);
>         >  memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
>         > sizeof(struct timeval));
>         > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
>         > pkt_tbl[], unsigned len)
>         >  "ICMP Echo Reply seq %d time %.1f ",
>         >  odp_be_to_cpu_16(icmp->un.echo.sequence)
>         >                                       , rtt);
>         > -                     } else if (icmp->type == ICMP_ECHO) {
>         > +                     } else if (icmp->type == ODPH_ICMP_ECHO) {
>         >                               rlen += sprintf(msg + rlen,
>         >      "Icmp Echo Request");
>         >                       }
>         > diff --git a/helper/include/odph_chksum.h
>         b/helper/include/odph_chksum.h
>         > index 710711a..3bcc2b6 100644
>         > --- a/helper/include/odph_chksum.h
>         > +++ b/helper/include/odph_chksum.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * ODP Checksum
>         > + * ODPH Checksum
>         >   */
>         >  #ifndef ODP_CHKSUM_H_
>         >  #define ODP_CHKSUM_H_
>         > diff --git a/helper/include/odph_eth.h
>         b/helper/include/odph_eth.h
>         > index 55a2b1e..45f7fea 100644
>         > --- a/helper/include/odph_eth.h
>         > +++ b/helper/include/odph_eth.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * ODP ethernet header
>         > + * ODPH ethernet header
>         >   */
>         >
>         >  #ifndef ODPH_ETH_H_
>         > diff --git a/helper/include/odph_icmp.h
>         b/helper/include/odph_icmp.h
>         > index 8414d7e..622f48b 100644
>         > --- a/helper/include/odph_icmp.h
>         > +++ b/helper/include/odph_icmp.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * ODP ICMP header
>         > + * ODPH ICMP header
>         >   */
>         >
>         >  #ifndef ODPH_ICMP_H_
>         > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
>         >       } un;                   /**< icmp sub header */
>         >  } odph_icmphdr_t;
>         >
>         > -#define ICMP_ECHOREPLY               0      /**< Echo
>         Reply                 */
>         > -#define ICMP_DEST_UNREACH    3  /**< Destination
>         Unreachable    */
>         > -#define ICMP_SOURCE_QUENCH   4  /**< Source Quench         
>             */
>         > -#define ICMP_REDIRECT                5      /**< Redirect
>         (change route)    */
>         > -#define ICMP_ECHO            8  /**< Echo Request         
>              */
>         > -#define ICMP_TIME_EXCEEDED   11 /**< Time Exceeded         
>             */
>         > -#define ICMP_PARAMETERPROB   12 /**< Parameter Problem     
>             */
>         > -#define ICMP_TIMESTAMP               13     /**< Timestamp
>         Request          */
>         > -#define ICMP_TIMESTAMPREPLY  14 /**< Timestamp Reply       
>             */
>         > -#define ICMP_INFO_REQUEST    15 /**< Information Request   
>             */
>         > -#define ICMP_INFO_REPLY              16     /**<
>         Information Reply          */
>         > -#define ICMP_ADDRESS         17 /**< Address Mask Request 
>              */
>         > -#define ICMP_ADDRESSREPLY    18 /**< Address Mask Reply   
>              */
>         > -#define NR_ICMP_TYPES                18     /**< Number of
>         icmp types       */
>         > +#define ODPH_ICMP_ECHOREPLY          0 /**< Echo Reply */
>         > +#define ODPH_ICMP_DEST_UNREACH      3  /**< Destination
>         Unreachable
>         > */
>         > +#define ODPH_ICMP_SOURCE_QUENCH     4  /**< Source Quench */
>         > +#define ODPH_ICMP_REDIRECT           5 /**< Redirect
>         (change route) */
>         > +#define ODPH_ICMP_ECHO      8  /**< Echo Request */
>         > +#define ODPH_ICMP_TIME_EXCEEDED     11 /**< Time Exceeded */
>         > +#define ODPH_ICMP_PARAMETERPROB     12 /**< Parameter
>         Problem */
>         > +#define ODPH_ICMP_TIMESTAMP          13 /**< Timestamp
>         Request */
>         > +#define ODPH_ICMP_TIMESTAMPREPLY     14 /**< Timestamp Reply */
>         > +#define ODPH_ICMP_INFO_REQUEST      15 /**< Information
>         Request */
>         > +#define ODPH_ICMP_INFO_REPLY         16 /**< Information
>         Reply */
>         > +#define ODPH_ICMP_ADDRESS            17 /**< Address Mask
>         Request */
>         > +#define ODPH_ICMP_ADDRESSREPLY      18 /**< Address Mask
>         Reply */
>         > +#define ODPH_NR_ICMP_TYPES           18 /**< Number of icmp
>         types */
>         >
>         >  /** Codes for UNREACH. */
>         > -#define ICMP_NET_UNREACH     0  /**< Network Unreachable   
>             */
>         > -#define ICMP_HOST_UNREACH    1  /**< Host Unreachable     
>              */
>         > -#define ICMP_PROT_UNREACH    2  /**< Protocol Unreachable 
>              */
>         > -#define ICMP_PORT_UNREACH    3  /**< Port Unreachable     
>              */
>         > -#define ICMP_FRAG_NEEDED     4  /**< Fragmentation
>         Needed/DF set*/
>         > -#define ICMP_SR_FAILED               5      /**< Source
>         Route failed        */
>         > -#define ICMP_NET_UNKNOWN     6  /**< Network Unknown       
>             */
>         > -#define ICMP_HOST_UNKNOWN    7  /**< Host Unknown         
>              */
>         > -#define ICMP_HOST_ISOLATED   8  /**< Host Isolated         
>             */
>         > -#define ICMP_NET_ANO         9  /**< ICMP_NET_ANO         
>              */
>         > -#define ICMP_HOST_ANO                10     /**<
>         ICMP_HOST_ANO              */
>         > -#define ICMP_NET_UNR_TOS     11 /**< ICMP_NET_UNR_TOS     
>              */
>         > -#define ICMP_HOST_UNR_TOS    12 /**< ICMP_HOST_UNR_TOS     
>             */
>         > -#define ICMP_PKT_FILTERED    13 /**< Packet filtered       
>             */
>         > -#define ICMP_PREC_VIOLATION  14 /**< Precedence violation 
>              */
>         > -#define ICMP_PREC_CUTOFF     15 /**< Precedence cut off   
>              */
>         > -#define NR_ICMP_UNREACH              15     /**< instead of
>         hardcoding
>         > -              immediate value */
>         > +#define ODPH_ICMP_NET_UNREACH     0  /**< Network
>         Unreachable */
>         > +#define ODPH_ICMP_HOST_UNREACH      1  /**< Host Unreachable */
>         > +#define ODPH_ICMP_PROT_UNREACH      2  /**< Protocol
>         Unreachable */
>         > +#define ODPH_ICMP_PORT_UNREACH      3  /**< Port Unreachable */
>         > +#define ODPH_ICMP_FRAG_NEEDED     4  /**< Fragmentation
>         Needed/DF
>         > set */
>         > +#define ODPH_ICMP_SR_FAILED          5 /**< Source Route
>         failed */
>         > +#define ODPH_ICMP_NET_UNKNOWN     6  /**< Network Unknown */
>         > +#define ODPH_ICMP_HOST_UNKNOWN      7  /**< Host Unknown */
>         > +#define ODPH_ICMP_HOST_ISOLATED     8  /**< Host Isolated */
>         > +#define ODPH_ICMP_NET_ANO            9 /**< ICMP_NET_ANO */
>         > +#define ODPH_ICMP_HOST_ANO           10 /**< ICMP_HOST_ANO */
>         > +#define ODPH_ICMP_NET_UNR_TOS     11 /**< ICMP_NET_UNR_TOS */
>         > +#define ODPH_ICMP_HOST_UNR_TOS      12 /**<
>         ICMP_HOST_UNR_TOS */
>         > +#define ODPH_ICMP_PKT_FILTERED      13 /**< Packet filtered */
>         > +#define ODPH_ICMP_PREC_VIOLATION     14 /**< Precedence
>         violation */
>         > +#define ODPH_ICMP_PREC_CUTOFF     15 /**< Precedence cut off */
>         > +#define ODPH_NR_ICMP_UNREACH         15 /**< instead of
>         hardcoding
>         > +      immediate value */
>         >
>         >  /** Codes for REDIRECT. */
>         > -#define ICMP_REDIR_NET               0      /**< Redirect
>         Net               */
>         > -#define ICMP_REDIR_HOST              1      /**< Redirect
>         Host              */
>         > -#define ICMP_REDIR_NETTOS    2  /**< Redirect Net for TOS 
>              */
>         > -#define ICMP_REDIR_HOSTTOS   3  /**< Redirect Host for TOS 
>             */
>         > +#define ODPH_ICMP_REDIR_NET          0 /**< Redirect Net */
>         > +#define ODPH_ICMP_REDIR_HOST         1 /**< Redirect Host */
>         > +#define ODPH_ICMP_REDIR_NETTOS      2  /**< Redirect Net
>         for TOS */
>         > +#define ODPH_ICMP_REDIR_HOSTTOS     3  /**< Redirect Host
>         for TOS */
>         >
>         >  /** Codes for TIME_EXCEEDED. */
>         > -#define ICMP_EXC_TTL         0  /**< TTL count exceeded   
>              */
>         > -#define ICMP_EXC_FRAGTIME    1  /**< Fragment Reass time
>         > -                      exceeded*/
>         > +#define ODPH_ICMP_EXC_TTL            0 /**< TTL count
>         exceeded */
>         > +#define ODPH_ICMP_EXC_FRAGTIME      1  /**< Fragment Reass time
>         > exceeded*/
>         >
>         >  /** @internal Compile time assert */
>         >  ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
>         > "ODPH_ICMPHDR_T__SIZE_ERROR");
>         > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
>         > index ca71c44..4fb9c12 100644
>         > --- a/helper/include/odph_ip.h
>         > +++ b/helper/include/odph_ip.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * ODP IP header
>         > + * ODPH IP header
>         >   */
>         >
>         >  #ifndef ODPH_IP_H_
>         > diff --git a/helper/include/odph_linux.h
>         b/helper/include/odph_linux.h
>         > index 1ea349a..be91dd3 100644
>         > --- a/helper/include/odph_linux.h
>         > +++ b/helper/include/odph_linux.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * ODP Linux helper API
>         > + * ODPH Linux helper API
>         >   *
>         >   * This file is an optional helper to odp.h APIs.
>         Application can manage
>         >   * pthreads also by other means.
>         > diff --git a/helper/include/odph_packet_helper.h
>         > b/helper/include/odph_packet_helper.h
>         > index c18f48d..c5315a9 100644
>         > --- a/helper/include/odph_packet_helper.h
>         > +++ b/helper/include/odph_packet_helper.h
>         > @@ -8,7 +8,7 @@
>         >  /**
>         >   * @file
>         >   *
>         > - * Optional ODP packet helper functions
>         > + * Optional ODPH packet helper functions
>         >   */
>         >
>         >  #ifndef ODPH_PACKET_HELPER_H_
>         > diff --git a/helper/include/odph_ring.h
>         b/helper/include/odph_ring.h
>         > index 76c1db8..82d2d13 100644
>         > --- a/helper/include/odph_ring.h
>         > +++ b/helper/include/odph_ring.h
>         > @@ -192,7 +192,7 @@ typedef struct odph_ring {
>         >   *    - ENOMEM - no appropriate memory area found in which
>         to create
>         > memzone
>         >   */
>         >  odph_ring_t *odph_ring_create(const char *name, unsigned count,
>         > -                         unsigned flags);
>         > +                           unsigned flags);
>         >
>         >
>         >  /**
>         > --
>         > 1.9.1
>         >
>         >
>         > _______________________________________________
>         > lng-odp mailing list
>         > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>         > http://lists.linaro.org/mailman/listinfo/lng-odp
>
>         _______________________________________________
>         lng-odp mailing list
>         lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>         http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> -- 
> *Mike Holmes*
> Linaro Technical Manager / Lead
> LNG - ODP
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Anders Roxell Sept. 4, 2014, 7:48 p.m. UTC | #9

On 2014-09-01 14:41, Maxim Uvarov wrote:
> My point is that defines are unrolled on preprocessor stage. While
> name spaces on link stage.
> So that if you do not change define value you can have several
> defines for that in different files.
> Gcc will not warn on that.  Only if the same define will be defined
> with different name.
> 
> Example:
> odp.h:
> #define AAA 1
> #define AAA 1
> 
> Will not warn, and it's valid.
> 
> I do not see the reason why we have to change define for ICMP ECHO
> bit to some odp specific. While in fact we will not change it
> in future.
> 
> Does it sound reasonable?

Feels dangerous, that means that one application with two code branches
can use the same define they think... however, that can be defined with
different values right?

If not, then we can drop this patch, but then we need to remove ODPH_
from all the defines that we have in ODP right?

Cheers,
Anders

> 
> Best regards,
> Maxim.
> 
> On 08/29/2014 10:10 PM, Mike Holmes wrote:
> >Maxim,
> >
> >Does this impact interworking with other packages like SNORT etc
> >in your experience ?
> >Is it likely that someone will need to convert to generic headers,
> >do you have a use case that this might be an issue?
> >
> >I think we need to scope the defines, but would like to be sure we
> >are not making a lot of peoples lives difficult.
> >
> >Mike
> >
> >
> >On 29 August 2014 08:21, Bill Fischofer <bill.fischofer@linaro.org
> ><mailto:bill.fischofer@linaro.org>> wrote:
> >
> >    I agree with Petri. Since C has a single global namespace for
> >    #defines the real issue is name collisions with applications using
> >    ODP.
> >
> >    Bill
> >
> >
> >    On Fri, Aug 29, 2014 at 6:49 AM, Savolainen, Petri (NSN -
> >    FI/Espoo) <petri.savolainen@nsn.com
> >    <mailto:petri.savolainen@nsn.com>> wrote:
> >
> >
> >
> >        > -----Original Message-----
> >        > From: lng-odp-bounces@lists.linaro.org
> >        <mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-
> >        <mailto:lng-odp->
> >        > bounces@lists.linaro.org <mailto:bounces@lists.linaro.org>]
> >        On Behalf Of ext Anders Roxell
> >        > Sent: Friday, August 29, 2014 1:02 AM
> >        > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
> >        > Subject: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> >        >
> >        > Signed-off-by: Anders Roxell <anders.roxell@linaro.org
> >        <mailto:anders.roxell@linaro.org>>
> >        Acked-by: Petri Savolainen <petri.savolainen@linaro.org
> >        <mailto:petri.savolainen@linaro.org>>
> >        > ---
> >        >  example/generator/odp_generator.c   |  6 +--
> >        >  helper/include/odph_chksum.h        |  2 +-
> >        >  helper/include/odph_eth.h           |  2 +-
> >        >  helper/include/odph_icmp.h          | 79
> >        ++++++++++++++++++--------------
> >        > -----
> >        >  helper/include/odph_ip.h            |  2 +-
> >        >  helper/include/odph_linux.h         |  2 +-
> >        >  helper/include/odph_packet_helper.h |  2 +-
> >        >  helper/include/odph_ring.h          |  2 +-
> >        >  8 files changed, 48 insertions(+), 49 deletions(-)
> >        >
> >        > diff --git a/example/generator/odp_generator.c
> >        > b/example/generator/odp_generator.c
> >        > index 70c0353..246bccf 100644
> >        > --- a/example/generator/odp_generator.c
> >        > +++ b/example/generator/odp_generator.c
> >        > @@ -264,7 +264,7 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
> >        >       odph_ipv4_csum_update(pkt);
> >        >       /* icmp */
> >        >       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +
> >        ODPH_IPV4HDR_LEN);
> >        > -     icmp->type = ICMP_ECHO;
> >        > +     icmp->type = ODPH_ICMP_ECHO;
> >        >       icmp->code = 0;
> >        >       icmp->un.echo.id <http://un.echo.id> = 0;
> >        >       icmp->un.echo.sequence = ip->id;
> >        > @@ -419,7 +419,7 @@ static void print_pkts(int thr, odp_packet_t
> >        > pkt_tbl[], unsigned len)
> >        >               if (ip->proto == ODPH_IPPROTO_ICMP) {
> >        >                       icmp = (odph_icmphdr_t *)(buf + offset);
> >        >                       /* echo reply */
> >        > -                     if (icmp->type == ICMP_ECHOREPLY) {
> >        > +                     if (icmp->type == ODPH_ICMP_ECHOREPLY) {
> >        >  odp_atomic_inc_u64(&counters.icmp);
> >        >  memcpy(&tvsend, buf + offset + ODPH_ICMPHDR_LEN,
> >        > sizeof(struct timeval));
> >        > @@ -432,7 +432,7 @@ static void print_pkts(int thr, odp_packet_t
> >        > pkt_tbl[], unsigned len)
> >        >  "ICMP Echo Reply seq %d time %.1f ",
> >        >  odp_be_to_cpu_16(icmp->un.echo.sequence)
> >        >                                       , rtt);
> >        > -                     } else if (icmp->type == ICMP_ECHO) {
> >        > +                     } else if (icmp->type == ODPH_ICMP_ECHO) {
> >        >                               rlen += sprintf(msg + rlen,
> >        >      "Icmp Echo Request");
> >        >                       }
> >        > diff --git a/helper/include/odph_chksum.h
> >        b/helper/include/odph_chksum.h
> >        > index 710711a..3bcc2b6 100644
> >        > --- a/helper/include/odph_chksum.h
> >        > +++ b/helper/include/odph_chksum.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * ODP Checksum
> >        > + * ODPH Checksum
> >        >   */
> >        >  #ifndef ODP_CHKSUM_H_
> >        >  #define ODP_CHKSUM_H_
> >        > diff --git a/helper/include/odph_eth.h
> >        b/helper/include/odph_eth.h
> >        > index 55a2b1e..45f7fea 100644
> >        > --- a/helper/include/odph_eth.h
> >        > +++ b/helper/include/odph_eth.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * ODP ethernet header
> >        > + * ODPH ethernet header
> >        >   */
> >        >
> >        >  #ifndef ODPH_ETH_H_
> >        > diff --git a/helper/include/odph_icmp.h
> >        b/helper/include/odph_icmp.h
> >        > index 8414d7e..622f48b 100644
> >        > --- a/helper/include/odph_icmp.h
> >        > +++ b/helper/include/odph_icmp.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * ODP ICMP header
> >        > + * ODPH ICMP header
> >        >   */
> >        >
> >        >  #ifndef ODPH_ICMP_H_
> >        > @@ -43,51 +43,50 @@ typedef struct ODPH_PACKED {
> >        >       } un;                   /**< icmp sub header */
> >        >  } odph_icmphdr_t;
> >        >
> >        > -#define ICMP_ECHOREPLY               0      /**< Echo
> >        Reply                 */
> >        > -#define ICMP_DEST_UNREACH    3  /**< Destination
> >        Unreachable    */
> >        > -#define ICMP_SOURCE_QUENCH   4  /**< Source Quench
> >*/
> >        > -#define ICMP_REDIRECT                5      /**< Redirect
> >        (change route)    */
> >        > -#define ICMP_ECHO            8  /**< Echo Request
> >*/
> >        > -#define ICMP_TIME_EXCEEDED   11 /**< Time Exceeded
> >*/
> >        > -#define ICMP_PARAMETERPROB   12 /**< Parameter Problem
> >*/
> >        > -#define ICMP_TIMESTAMP               13     /**< Timestamp
> >        Request          */
> >        > -#define ICMP_TIMESTAMPREPLY  14 /**< Timestamp Reply
> >*/
> >        > -#define ICMP_INFO_REQUEST    15 /**< Information
> >Request               */
> >        > -#define ICMP_INFO_REPLY              16     /**<
> >        Information Reply          */
> >        > -#define ICMP_ADDRESS         17 /**< Address Mask
> >Request              */
> >        > -#define ICMP_ADDRESSREPLY    18 /**< Address Mask Reply
> >*/
> >        > -#define NR_ICMP_TYPES                18     /**< Number of
> >        icmp types       */
> >        > +#define ODPH_ICMP_ECHOREPLY          0 /**< Echo Reply */
> >        > +#define ODPH_ICMP_DEST_UNREACH      3  /**< Destination
> >        Unreachable
> >        > */
> >        > +#define ODPH_ICMP_SOURCE_QUENCH     4  /**< Source Quench */
> >        > +#define ODPH_ICMP_REDIRECT           5 /**< Redirect
> >        (change route) */
> >        > +#define ODPH_ICMP_ECHO      8  /**< Echo Request */
> >        > +#define ODPH_ICMP_TIME_EXCEEDED     11 /**< Time Exceeded */
> >        > +#define ODPH_ICMP_PARAMETERPROB     12 /**< Parameter
> >        Problem */
> >        > +#define ODPH_ICMP_TIMESTAMP          13 /**< Timestamp
> >        Request */
> >        > +#define ODPH_ICMP_TIMESTAMPREPLY     14 /**< Timestamp Reply */
> >        > +#define ODPH_ICMP_INFO_REQUEST      15 /**< Information
> >        Request */
> >        > +#define ODPH_ICMP_INFO_REPLY         16 /**< Information
> >        Reply */
> >        > +#define ODPH_ICMP_ADDRESS            17 /**< Address Mask
> >        Request */
> >        > +#define ODPH_ICMP_ADDRESSREPLY      18 /**< Address Mask
> >        Reply */
> >        > +#define ODPH_NR_ICMP_TYPES           18 /**< Number of icmp
> >        types */
> >        >
> >        >  /** Codes for UNREACH. */
> >        > -#define ICMP_NET_UNREACH     0  /**< Network
> >Unreachable               */
> >        > -#define ICMP_HOST_UNREACH    1  /**< Host Unreachable
> >*/
> >        > -#define ICMP_PROT_UNREACH    2  /**< Protocol
> >Unreachable              */
> >        > -#define ICMP_PORT_UNREACH    3  /**< Port Unreachable
> >*/
> >        > -#define ICMP_FRAG_NEEDED     4  /**< Fragmentation
> >        Needed/DF set*/
> >        > -#define ICMP_SR_FAILED               5      /**< Source
> >        Route failed        */
> >        > -#define ICMP_NET_UNKNOWN     6  /**< Network Unknown
> >*/
> >        > -#define ICMP_HOST_UNKNOWN    7  /**< Host Unknown
> >*/
> >        > -#define ICMP_HOST_ISOLATED   8  /**< Host Isolated
> >*/
> >        > -#define ICMP_NET_ANO         9  /**< ICMP_NET_ANO
> >*/
> >        > -#define ICMP_HOST_ANO                10     /**<
> >        ICMP_HOST_ANO              */
> >        > -#define ICMP_NET_UNR_TOS     11 /**< ICMP_NET_UNR_TOS
> >*/
> >        > -#define ICMP_HOST_UNR_TOS    12 /**< ICMP_HOST_UNR_TOS
> >*/
> >        > -#define ICMP_PKT_FILTERED    13 /**< Packet filtered
> >*/
> >        > -#define ICMP_PREC_VIOLATION  14 /**< Precedence
> >violation              */
> >        > -#define ICMP_PREC_CUTOFF     15 /**< Precedence cut off
> >*/
> >        > -#define NR_ICMP_UNREACH              15     /**< instead of
> >        hardcoding
> >        > -              immediate value */
> >        > +#define ODPH_ICMP_NET_UNREACH     0  /**< Network
> >        Unreachable */
> >        > +#define ODPH_ICMP_HOST_UNREACH      1  /**< Host Unreachable */
> >        > +#define ODPH_ICMP_PROT_UNREACH      2  /**< Protocol
> >        Unreachable */
> >        > +#define ODPH_ICMP_PORT_UNREACH      3  /**< Port Unreachable */
> >        > +#define ODPH_ICMP_FRAG_NEEDED     4  /**< Fragmentation
> >        Needed/DF
> >        > set */
> >        > +#define ODPH_ICMP_SR_FAILED          5 /**< Source Route
> >        failed */
> >        > +#define ODPH_ICMP_NET_UNKNOWN     6  /**< Network Unknown */
> >        > +#define ODPH_ICMP_HOST_UNKNOWN      7  /**< Host Unknown */
> >        > +#define ODPH_ICMP_HOST_ISOLATED     8  /**< Host Isolated */
> >        > +#define ODPH_ICMP_NET_ANO            9 /**< ICMP_NET_ANO */
> >        > +#define ODPH_ICMP_HOST_ANO           10 /**< ICMP_HOST_ANO */
> >        > +#define ODPH_ICMP_NET_UNR_TOS     11 /**< ICMP_NET_UNR_TOS */
> >        > +#define ODPH_ICMP_HOST_UNR_TOS      12 /**<
> >        ICMP_HOST_UNR_TOS */
> >        > +#define ODPH_ICMP_PKT_FILTERED      13 /**< Packet filtered */
> >        > +#define ODPH_ICMP_PREC_VIOLATION     14 /**< Precedence
> >        violation */
> >        > +#define ODPH_ICMP_PREC_CUTOFF     15 /**< Precedence cut off */
> >        > +#define ODPH_NR_ICMP_UNREACH         15 /**< instead of
> >        hardcoding
> >        > +      immediate value */
> >        >
> >        >  /** Codes for REDIRECT. */
> >        > -#define ICMP_REDIR_NET               0      /**< Redirect
> >        Net               */
> >        > -#define ICMP_REDIR_HOST              1      /**< Redirect
> >        Host              */
> >        > -#define ICMP_REDIR_NETTOS    2  /**< Redirect Net for
> >TOS              */
> >        > -#define ICMP_REDIR_HOSTTOS   3  /**< Redirect Host for
> >TOS             */
> >        > +#define ODPH_ICMP_REDIR_NET          0 /**< Redirect Net */
> >        > +#define ODPH_ICMP_REDIR_HOST         1 /**< Redirect Host */
> >        > +#define ODPH_ICMP_REDIR_NETTOS      2  /**< Redirect Net
> >        for TOS */
> >        > +#define ODPH_ICMP_REDIR_HOSTTOS     3  /**< Redirect Host
> >        for TOS */
> >        >
> >        >  /** Codes for TIME_EXCEEDED. */
> >        > -#define ICMP_EXC_TTL         0  /**< TTL count exceeded
> >*/
> >        > -#define ICMP_EXC_FRAGTIME    1  /**< Fragment Reass time
> >        > -                      exceeded*/
> >        > +#define ODPH_ICMP_EXC_TTL            0 /**< TTL count
> >        exceeded */
> >        > +#define ODPH_ICMP_EXC_FRAGTIME      1  /**< Fragment Reass time
> >        > exceeded*/
> >        >
> >        >  /** @internal Compile time assert */
> >        >  ODP_STATIC_ASSERT(sizeof(odph_icmphdr_t) == ODPH_ICMPHDR_LEN,
> >        > "ODPH_ICMPHDR_T__SIZE_ERROR");
> >        > diff --git a/helper/include/odph_ip.h b/helper/include/odph_ip.h
> >        > index ca71c44..4fb9c12 100644
> >        > --- a/helper/include/odph_ip.h
> >        > +++ b/helper/include/odph_ip.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * ODP IP header
> >        > + * ODPH IP header
> >        >   */
> >        >
> >        >  #ifndef ODPH_IP_H_
> >        > diff --git a/helper/include/odph_linux.h
> >        b/helper/include/odph_linux.h
> >        > index 1ea349a..be91dd3 100644
> >        > --- a/helper/include/odph_linux.h
> >        > +++ b/helper/include/odph_linux.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * ODP Linux helper API
> >        > + * ODPH Linux helper API
> >        >   *
> >        >   * This file is an optional helper to odp.h APIs.
> >        Application can manage
> >        >   * pthreads also by other means.
> >        > diff --git a/helper/include/odph_packet_helper.h
> >        > b/helper/include/odph_packet_helper.h
> >        > index c18f48d..c5315a9 100644
> >        > --- a/helper/include/odph_packet_helper.h
> >        > +++ b/helper/include/odph_packet_helper.h
> >        > @@ -8,7 +8,7 @@
> >        >  /**
> >        >   * @file
> >        >   *
> >        > - * Optional ODP packet helper functions
> >        > + * Optional ODPH packet helper functions
> >        >   */
> >        >
> >        >  #ifndef ODPH_PACKET_HELPER_H_
> >        > diff --git a/helper/include/odph_ring.h
> >        b/helper/include/odph_ring.h
> >        > index 76c1db8..82d2d13 100644
> >        > --- a/helper/include/odph_ring.h
> >        > +++ b/helper/include/odph_ring.h
> >        > @@ -192,7 +192,7 @@ typedef struct odph_ring {
> >        >   *    - ENOMEM - no appropriate memory area found in which
> >        to create
> >        > memzone
> >        >   */
> >        >  odph_ring_t *odph_ring_create(const char *name, unsigned count,
> >        > -                         unsigned flags);
> >        > +                           unsigned flags);
> >        >
> >        >
> >        >  /**
> >        > --
> >        > 1.9.1
> >        >
> >        >
> >        > _______________________________________________
> >        > lng-odp mailing list
> >        > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
> >        > http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >        _______________________________________________
> >        lng-odp mailing list
> >        lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
> >        http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> >    _______________________________________________
> >    lng-odp mailing list
> >    lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
> >    http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> >
> >-- 
> >*Mike Holmes*
> >Linaro Technical Manager / Lead
> >LNG - ODP
> >
> >
> >_______________________________________________
> >lng-odp mailing list
> >lng-odp@lists.linaro.org
> >http://lists.linaro.org/mailman/listinfo/lng-odp
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Maxim Uvarov Sept. 6, 2014, 8:56 p.m. UTC | #10

On 09/04/2014 11:48 PM, Anders Roxell wrote:
> Feels dangerous, that means that one application with two code branches
> can use the same define they think... however, that can be defined with
> different values right?

Not. If they will define to different values then you will see 
compilation error.
Gcc will say that you are trying to redefine value.

> If not, then we can drop this patch, but then we need to remove ODPH_
> from all the defines that we have in ODP right?
Need to walk over them. If it's standard system values, then we don't 
need to redefine them.
We should be very careful with changes due to changing this things 
require changes in all odp
applications.

Maxim.

>
> Cheers,
> Anders

Savolainen, Petri (NSN - FI/Espoo) Sept. 8, 2014, 8:10 a.m. UTC | #11

> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
> Sent: Saturday, September 06, 2014 11:56 PM
> To: Anders Roxell
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> 
> On 09/04/2014 11:48 PM, Anders Roxell wrote:
> > Feels dangerous, that means that one application with two code branches
> > can use the same define they think... however, that can be defined with
> > different values right?
> 
> Not. If they will define to different values then you will see
> compilation error.
> Gcc will say that you are trying to redefine value.
> 
> > If not, then we can drop this patch, but then we need to remove ODPH_
> > from all the defines that we have in ODP right?
> Need to walk over them. If it's standard system values, then we don't
> need to redefine them.
> We should be very careful with changes due to changing this things
> require changes in all odp
> applications.
> 

All ODP helper definitions should have ODPH_ or odph_ prefix. Those are only helpers - there's no guarantees (== limited SW definitions and maintenance) to support a full featured protocol implementation. Such an implementation would be another project then.

Also what are standard system values (definitions)? Defined in POSIX/Linux/Unix/RTOS X/Vendor SDK Y/IETF RFC Z?

-Petri

> Maxim.
> 
> >
> > Cheers,
> > Anders
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Maxim Uvarov Sept. 8, 2014, 11:13 a.m. UTC | #12

On 09/08/2014 12:10 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
>
>> -----Original Message-----
>> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
>> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
>> Sent: Saturday, September 06, 2014 11:56 PM
>> To: Anders Roxell
>> Cc: lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
>>
>> On 09/04/2014 11:48 PM, Anders Roxell wrote:
>>> Feels dangerous, that means that one application with two code branches
>>> can use the same define they think... however, that can be defined with
>>> different values right?
>> Not. If they will define to different values then you will see
>> compilation error.
>> Gcc will say that you are trying to redefine value.
>>
>>> If not, then we can drop this patch, but then we need to remove ODPH_
>>> from all the defines that we have in ODP right?
>> Need to walk over them. If it's standard system values, then we don't
>> need to redefine them.
>> We should be very careful with changes due to changing this things
>> require changes in all odp
>> applications.
>>
> All ODP helper definitions should have ODPH_ or odph_ prefix. Those are only helpers - there's no guarantees (== limited SW definitions and maintenance) to support a full featured protocol implementation. Such an implementation would be another project then.
>
> Also what are standard system values (definitions)? Defined in POSIX/Linux/Unix/RTOS X/Vendor SDK Y/IETF RFC Z?
>
> -Petri

It does not matter where they defined. Important thing that they do not 
change, like:

-#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
-#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
-#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
-#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
-#define ICMP_INFO_REQUEST	15	/**< Information Request	*/

Do you now where this values are different?

Thanks,
Maxim.


>
>> Maxim.
>>
>>> Cheers,
>>> Anders
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp

Savolainen, Petri (NSN - FI/Espoo) Sept. 8, 2014, 11:31 a.m. UTC | #13

> -----Original Message-----
> From: ext Maxim Uvarov [mailto:maxim.uvarov@linaro.org]
> Sent: Monday, September 08, 2014 2:13 PM
> To: Savolainen, Petri (NSN - FI/Espoo); Anders Roxell
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> 
> On 09/08/2014 12:10 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
> >
> >> -----Original Message-----
> >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> >> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov
> >> Sent: Saturday, September 06, 2014 11:56 PM
> >> To: Anders Roxell
> >> Cc: lng-odp@lists.linaro.org
> >> Subject: Re: [lng-odp] [PATCH 2/2] odph_icmp: add ODPH_ prefix
> >>
> >> On 09/04/2014 11:48 PM, Anders Roxell wrote:
> >>> Feels dangerous, that means that one application with two code
> branches
> >>> can use the same define they think... however, that can be defined
> with
> >>> different values right?
> >> Not. If they will define to different values then you will see
> >> compilation error.
> >> Gcc will say that you are trying to redefine value.
> >>
> >>> If not, then we can drop this patch, but then we need to remove ODPH_
> >>> from all the defines that we have in ODP right?
> >> Need to walk over them. If it's standard system values, then we don't
> >> need to redefine them.
> >> We should be very careful with changes due to changing this things
> >> require changes in all odp
> >> applications.
> >>
> > All ODP helper definitions should have ODPH_ or odph_ prefix. Those are
> only helpers - there's no guarantees (== limited SW definitions and
> maintenance) to support a full featured protocol implementation. Such an
> implementation would be another project then.
> >
> > Also what are standard system values (definitions)? Defined in
> POSIX/Linux/Unix/RTOS X/Vendor SDK Y/IETF RFC Z?
> >
> > -Petri
> 
> It does not matter where they defined. Important thing that they do not
> change, like:
> 
> -#define ICMP_TIME_EXCEEDED	11	/**< Time Exceeded		*/
> -#define ICMP_PARAMETERPROB	12	/**< Parameter Problem		*/
> -#define ICMP_TIMESTAMP		13	/**< Timestamp Request		*/
> -#define ICMP_TIMESTAMPREPLY	14	/**< Timestamp Reply		*/
> -#define ICMP_INFO_REQUEST	15	/**< Information Request	*/
> 
> Do you now where this values are different?
> 

How user can be sure that e.g. ICMP_TIMESTAMP is defined only for ICMP timestamp request operation code in system XYZ (which may not be Linux) and not for something else? With ODPH_ we can ensure that. Application is not portable if it does not build (due to name space clash).

-Petri


> Thanks,
> Maxim.
> 
> 
> >
> >> Maxim.
> >>
> >>> Cheers,
> >>> Anders
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp

[2/2] odph_icmp: add ODPH_ prefix

Commit Message

Comments

Patch