All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] examples/ip_fragmentation: add fragmentation size support
@ 2017-04-20  7:18 Ashish Jain
  2017-05-24  7:28 ` Ashish Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ashish Jain @ 2017-04-20  7:18 UTC (permalink / raw)
  To: dev, konstantin.ananyev

Adding support for determining fragmentation size for both
ipv4 and ipv6 traffic dynamically through command line.
It is helpful in testing to configure different fragmentation
sizes and validate the packets.

Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 examples/ip_fragmentation/main.c | 89 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 8 deletions(-)

diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
index 815b225..436755b 100644
--- a/examples/ip_fragmentation/main.c
+++ b/examples/ip_fragmentation/main.c
@@ -94,6 +94,16 @@
 #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
 
 /*
+ * Configure fragmentation size for IPv4 and IPv6 packets
+ */
+static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
+static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
+#define MIN_IPV4_FRAG_SIZE 64
+#define MAX_IPV4_FRAG_SIZE 9600
+#define MIN_IPV6_FRAG_SIZE 1280
+#define MAX_IPV6_FRAG_SIZE 9600
+
+/*
  * Max number of fragments per packet expected - defined by config file.
  */
 #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
@@ -299,14 +309,14 @@ struct rte_lpm6_config lpm6_config = {
 		}
 
 		/* if we don't need to do any fragmentation */
-		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
+		if (likely (frag_size_v4 >= m->pkt_len)) {
 			qconf->tx_mbufs[port_out].m_table[len] = m;
 			len2 = 1;
 		} else {
 			len2 = rte_ipv4_fragment_packet(m,
 				&qconf->tx_mbufs[port_out].m_table[len],
 				(uint16_t)(MBUF_TABLE_SIZE - len),
-				IPV4_MTU_DEFAULT,
+				frag_size_v4,
 				rxq->direct_pool, rxq->indirect_pool);
 
 			/* Free input packet */
@@ -336,14 +346,14 @@ struct rte_lpm6_config lpm6_config = {
 		}
 
 		/* if we don't need to do any fragmentation */
-		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
+		if (likely (frag_size_v6 >= m->pkt_len)) {
 			qconf->tx_mbufs[port_out].m_table[len] = m;
 			len2 = 1;
 		} else {
 			len2 = rte_ipv6_fragment_packet(m,
 				&qconf->tx_mbufs[port_out].m_table[len],
 				(uint16_t)(MBUF_TABLE_SIZE - len),
-				IPV6_MTU_DEFAULT,
+				frag_size_v6,
 				rxq->direct_pool, rxq->indirect_pool);
 
 			/* Free input packet */
@@ -489,8 +499,14 @@ struct rte_lpm6_config lpm6_config = {
 {
 	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
 	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
-	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
-	       prgname);
+	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
+	       "  --frag_size_v4=<num>:optional,IPv4 fragment size in decimal"
+	       ",Condition:(frag_size_v4 - 20) should be a multiple of 8,"
+	       " default is %d \n"
+	       "  --frag_size_v6=<num>:optional,IPv6 fragment size in decimal"
+	       ",Condition:(frag_size_v6 - 40) should be a multiple of 8,"
+	       " default is %d\n",
+	       prgname, frag_size_v4, frag_size_v6);
 }
 
 static int
@@ -528,6 +544,29 @@ struct rte_lpm6_config lpm6_config = {
 	return n;
 }
 
+static int
+parse_frag_size(const char *str, uint32_t min, uint32_t max,
+		uint8_t hdr_size, uint32_t *val)
+{
+	char *end;
+	uint64_t v;
+
+	/* parse decimal string */
+	errno = 0;
+	v = strtoul(str, &end, 10);
+	if (errno != 0 || *end != '\0')
+		return -EINVAL;
+
+	if (v < min || v > max)
+		return -EINVAL;
+
+	if ((v - hdr_size) % 8)
+		return -EINVAL;
+
+	*val = (uint32_t)v;
+	return 0;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 parse_args(int argc, char **argv)
@@ -537,6 +576,8 @@ struct rte_lpm6_config lpm6_config = {
 	int option_index;
 	char *prgname = argv[0];
 	static struct option lgopts[] = {
+		{"frag_size_v4", 1, 0, 0},
+		{"frag_size_v6", 1, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -568,8 +609,40 @@ struct rte_lpm6_config lpm6_config = {
 
 		/* long options */
 		case 0:
-			print_usage(prgname);
-			return -1;
+			if (!strncmp(lgopts[option_index].name,
+				"frag_size_v4", 12)) {
+				ret = parse_frag_size(optarg,
+						MIN_IPV4_FRAG_SIZE,
+						MAX_IPV4_FRAG_SIZE,
+						sizeof(struct ipv4_hdr),
+						&frag_size_v4);
+				if (ret) {
+					printf("invalid value: \"%s\" for "
+						"parameter %s\n",
+						optarg,
+						lgopts[option_index].name);
+					print_usage(prgname);
+					return ret;
+				}
+			}
+			if (!strncmp(lgopts[option_index].name,
+				"frag_size_v6", 12)) {
+				ret = parse_frag_size(optarg,
+						MIN_IPV6_FRAG_SIZE,
+						MAX_IPV6_FRAG_SIZE,
+						sizeof(struct ipv6_hdr),
+						&frag_size_v6);
+				if (ret) {
+					printf("invalid value: \"%s\" for "
+						"parameter %s\n",
+						optarg,
+						lgopts[option_index].name);
+					print_usage(prgname);
+					return ret;
+				}
+			}
+
+			break;
 
 		default:
 			print_usage(prgname);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] examples/ip_fragmentation: add fragmentation size support
  2017-04-20  7:18 [PATCH] examples/ip_fragmentation: add fragmentation size support Ashish Jain
@ 2017-05-24  7:28 ` Ashish Jain
  2017-06-04 15:44 ` Ananyev, Konstantin
  2017-06-30 12:18 ` [PATCH v2] example/ip_fragmentation: " Ashish Jain
  2 siblings, 0 replies; 9+ messages in thread
From: Ashish Jain @ 2017-05-24  7:28 UTC (permalink / raw)
  To: dev, konstantin.ananyev

Hi,

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ashish Jain
Sent: Thursday, April 20, 2017 12:48 PM
To: dev@dpdk.org; konstantin.ananyev@intel.com
Subject: [dpdk-dev] [PATCH] examples/ip_fragmentation: add fragmentation size support

Adding support for determining fragmentation size for both
ipv4 and ipv6 traffic dynamically through command line.
It is helpful in testing to configure different fragmentation sizes and validate the packets.

Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 examples/ip_fragmentation/main.c | 89 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 8 deletions(-)

diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
index 815b225..436755b 100644
--- a/examples/ip_fragmentation/main.c
+++ b/examples/ip_fragmentation/main.c
@@ -94,6 +94,16 @@
 #define        IPV6_DEFAULT_PAYLOAD    (IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))

 /*
+ * Configure fragmentation size for IPv4 and IPv6 packets  */ static 
+uint32_t frag_size_v4 = IPV4_MTU_DEFAULT; static uint32_t frag_size_v6 
+= IPV6_MTU_DEFAULT; #define MIN_IPV4_FRAG_SIZE 64 #define 
+MAX_IPV4_FRAG_SIZE 9600 #define MIN_IPV6_FRAG_SIZE 1280 #define 
+MAX_IPV6_FRAG_SIZE 9600
+
+/*
  * Max number of fragments per packet expected - defined by config file.
  */
 #define        MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
@@ -299,14 +309,14 @@ struct rte_lpm6_config lpm6_config = {
                }

                /* if we don't need to do any fragmentation */
-               if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
+               if (likely (frag_size_v4 >= m->pkt_len)) {
                        qconf->tx_mbufs[port_out].m_table[len] = m;
                        len2 = 1;
                } else {
                        len2 = rte_ipv4_fragment_packet(m,
                                &qconf->tx_mbufs[port_out].m_table[len],
                                (uint16_t)(MBUF_TABLE_SIZE - len),
-                               IPV4_MTU_DEFAULT,
+                               frag_size_v4,
                                rxq->direct_pool, rxq->indirect_pool);

                        /* Free input packet */ @@ -336,14 +346,14 @@ struct rte_lpm6_config lpm6_config = {
                }

                /* if we don't need to do any fragmentation */
-               if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
+               if (likely (frag_size_v6 >= m->pkt_len)) {
                        qconf->tx_mbufs[port_out].m_table[len] = m;
                        len2 = 1;
                } else {
                        len2 = rte_ipv6_fragment_packet(m,
                                &qconf->tx_mbufs[port_out].m_table[len],
                                (uint16_t)(MBUF_TABLE_SIZE - len),
-                               IPV6_MTU_DEFAULT,
+                               frag_size_v6,
                                rxq->direct_pool, rxq->indirect_pool);

                        /* Free input packet */ @@ -489,8 +499,14 @@ struct rte_lpm6_config lpm6_config = {  {
        printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
               "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
-              "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
-              prgname);
+              "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
+              "  --frag_size_v4=<num>:optional,IPv4 fragment size in decimal"
+              ",Condition:(frag_size_v4 - 20) should be a multiple of 8,"
+              " default is %d \n"
+              "  --frag_size_v6=<num>:optional,IPv6 fragment size in decimal"
+              ",Condition:(frag_size_v6 - 40) should be a multiple of 8,"
+              " default is %d\n",
+              prgname, frag_size_v4, frag_size_v6);
 }

 static int
@@ -528,6 +544,29 @@ struct rte_lpm6_config lpm6_config = {
        return n;
 }

+static int
+parse_frag_size(const char *str, uint32_t min, uint32_t max,
+               uint8_t hdr_size, uint32_t *val) {
+       char *end;
+       uint64_t v;
+
+       /* parse decimal string */
+       errno = 0;
+       v = strtoul(str, &end, 10);
+       if (errno != 0 || *end != '\0')
+               return -EINVAL;
+
+       if (v < min || v > max)
+               return -EINVAL;
+
+       if ((v - hdr_size) % 8)
+               return -EINVAL;
+
+       *val = (uint32_t)v;
+       return 0;
+}
+
 /* Parse the argument given in the command line of the application */  static int  parse_args(int argc, char **argv) @@ -537,6 +576,8 @@ struct rte_lpm6_config lpm6_config = {
        int option_index;
        char *prgname = argv[0];
        static struct option lgopts[] = {
+               {"frag_size_v4", 1, 0, 0},
+               {"frag_size_v6", 1, 0, 0},
                {NULL, 0, 0, 0}
        };

@@ -568,8 +609,40 @@ struct rte_lpm6_config lpm6_config = {

                /* long options */
                case 0:
-                       print_usage(prgname);
-                       return -1;
+                       if (!strncmp(lgopts[option_index].name,
+                               "frag_size_v4", 12)) {
+                               ret = parse_frag_size(optarg,
+                                               MIN_IPV4_FRAG_SIZE,
+                                               MAX_IPV4_FRAG_SIZE,
+                                               sizeof(struct ipv4_hdr),
+                                               &frag_size_v4);
+                               if (ret) {
+                                       printf("invalid value: \"%s\" for "
+                                               "parameter %s\n",
+                                               optarg,
+                                               lgopts[option_index].name);
+                                       print_usage(prgname);
+                                       return ret;
+                               }
+                       }
+                       if (!strncmp(lgopts[option_index].name,
+                               "frag_size_v6", 12)) {
+                               ret = parse_frag_size(optarg,
+                                               MIN_IPV6_FRAG_SIZE,
+                                               MAX_IPV6_FRAG_SIZE,
+                                               sizeof(struct ipv6_hdr),
+                                               &frag_size_v6);
+                               if (ret) {
+                                       printf("invalid value: \"%s\" for "
+                                               "parameter %s\n",
+                                               optarg,
+                                               lgopts[option_index].name);
+                                       print_usage(prgname);
+                                       return ret;
+                               }
+                       }
+
+                       break;

                default:
                        print_usage(prgname);
--
1.9.1

Any comments on this patch?

Cheers
Ashish

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] examples/ip_fragmentation: add fragmentation size support
  2017-04-20  7:18 [PATCH] examples/ip_fragmentation: add fragmentation size support Ashish Jain
  2017-05-24  7:28 ` Ashish Jain
@ 2017-06-04 15:44 ` Ananyev, Konstantin
  2017-06-14  9:19   ` Jain Ashish-B46179
  2017-06-30 12:18 ` [PATCH v2] example/ip_fragmentation: " Ashish Jain
  2 siblings, 1 reply; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-06-04 15:44 UTC (permalink / raw)
  To: Ashish Jain, dev

Hi, 

> -----Original Message-----
> From: Ashish Jain [mailto:ashish.jain@nxp.com]
> Sent: Thursday, April 20, 2017 8:18 AM
> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: [PATCH] examples/ip_fragmentation: add fragmentation size support
> 
> Adding support for determining fragmentation size for both
> ipv4 and ipv6 traffic dynamically through command line.
> It is helpful in testing to configure different fragmentation
> sizes and validate the packets.
> 
> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  examples/ip_fragmentation/main.c | 89 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 81 insertions(+), 8 deletions(-)
> 
> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> index 815b225..436755b 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -94,6 +94,16 @@
>  #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
> 
>  /*
> + * Configure fragmentation size for IPv4 and IPv6 packets
> + */
> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
> +#define MIN_IPV4_FRAG_SIZE 64
> +#define MAX_IPV4_FRAG_SIZE 9600
> +#define MIN_IPV6_FRAG_SIZE 1280
> +#define MAX_IPV6_FRAG_SIZE 9600
> +
> +/*
>   * Max number of fragments per packet expected - defined by config file.
>   */
>  #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
> @@ -299,14 +309,14 @@ struct rte_lpm6_config lpm6_config = {
>  		}
> 
>  		/* if we don't need to do any fragmentation */
> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
> +		if (likely (frag_size_v4 >= m->pkt_len)) {
>  			qconf->tx_mbufs[port_out].m_table[len] = m;
>  			len2 = 1;
>  		} else {
>  			len2 = rte_ipv4_fragment_packet(m,
>  				&qconf->tx_mbufs[port_out].m_table[len],
>  				(uint16_t)(MBUF_TABLE_SIZE - len),
> -				IPV4_MTU_DEFAULT,
> +				frag_size_v4,
>  				rxq->direct_pool, rxq->indirect_pool);
> 
>  			/* Free input packet */
> @@ -336,14 +346,14 @@ struct rte_lpm6_config lpm6_config = {
>  		}
> 
>  		/* if we don't need to do any fragmentation */
> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
> +		if (likely (frag_size_v6 >= m->pkt_len)) {
>  			qconf->tx_mbufs[port_out].m_table[len] = m;
>  			len2 = 1;
>  		} else {
>  			len2 = rte_ipv6_fragment_packet(m,
>  				&qconf->tx_mbufs[port_out].m_table[len],
>  				(uint16_t)(MBUF_TABLE_SIZE - len),
> -				IPV6_MTU_DEFAULT,
> +				frag_size_v6,
>  				rxq->direct_pool, rxq->indirect_pool);
> 
>  			/* Free input packet */
> @@ -489,8 +499,14 @@ struct rte_lpm6_config lpm6_config = {
>  {
>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>  	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
> -	       prgname);
> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> +	       "  --frag_size_v4=<num>:optional,IPv4 fragment size in decimal"
> +	       ",Condition:(frag_size_v4 - 20) should be a multiple of 8,"
> +	       " default is %d \n"
> +	       "  --frag_size_v6=<num>:optional,IPv6 fragment size in decimal"
> +	       ",Condition:(frag_size_v6 - 40) should be a multiple of 8,"
> +	       " default is %d\n",
> +	       prgname, frag_size_v4, frag_size_v6);
>  }
> 
>  static int
> @@ -528,6 +544,29 @@ struct rte_lpm6_config lpm6_config = {
>  	return n;
>  }
> 
> +static int
> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
> +		uint8_t hdr_size, uint32_t *val)
> +{
> +	char *end;
> +	uint64_t v;
> +
> +	/* parse decimal string */
> +	errno = 0;
> +	v = strtoul(str, &end, 10);
> +	if (errno != 0 || *end != '\0')
> +		return -EINVAL;
> +
> +	if (v < min || v > max)
> +		return -EINVAL;
> +
> +	if ((v - hdr_size) % 8)
> +		return -EINVAL;
> +
> +	*val = (uint32_t)v;
> +	return 0;
> +}
> +
>  /* Parse the argument given in the command line of the application */
>  static int
>  parse_args(int argc, char **argv)
> @@ -537,6 +576,8 @@ struct rte_lpm6_config lpm6_config = {
>  	int option_index;
>  	char *prgname = argv[0];
>  	static struct option lgopts[] = {
> +		{"frag_size_v4", 1, 0, 0},
> +		{"frag_size_v6", 1, 0, 0},
>  		{NULL, 0, 0, 0}
>  	};
> 
> @@ -568,8 +609,40 @@ struct rte_lpm6_config lpm6_config = {
> 
>  		/* long options */
>  		case 0:
> -			print_usage(prgname);
> -			return -1;
> +			if (!strncmp(lgopts[option_index].name,
> +				"frag_size_v4", 12)) {


Why just not strcmp() here?

> +				ret = parse_frag_size(optarg,
> +						MIN_IPV4_FRAG_SIZE,
> +						MAX_IPV4_FRAG_SIZE,
> +						sizeof(struct ipv4_hdr),
> +						&frag_size_v4);
> +				if (ret) {
> +					printf("invalid value: \"%s\" for "
> +						"parameter %s\n",
> +						optarg,
> +						lgopts[option_index].name);
> +					print_usage(prgname);
> +					return ret;
> +				}
> +			}
> +			if (!strncmp(lgopts[option_index].name,
> +				"frag_size_v6", 12)) {
> +				ret = parse_frag_size(optarg,
> +						MIN_IPV6_FRAG_SIZE,
> +						MAX_IPV6_FRAG_SIZE,
> +						sizeof(struct ipv6_hdr),
> +						&frag_size_v6);
> +				if (ret) {
> +					printf("invalid value: \"%s\" for "
> +						"parameter %s\n",
> +						optarg,
> +						lgopts[option_index].name);
> +					print_usage(prgname);
> +					return ret;
> +				}
> +			}
> +
> +			break;

Hmm, why do we need that break here?
Konstantin


> 
>  		default:
>  			print_usage(prgname);
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] examples/ip_fragmentation: add fragmentation size support
  2017-06-04 15:44 ` Ananyev, Konstantin
@ 2017-06-14  9:19   ` Jain Ashish-B46179
  2017-06-14 10:10     ` Ananyev, Konstantin
  0 siblings, 1 reply; 9+ messages in thread
From: Jain Ashish-B46179 @ 2017-06-14  9:19 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev

Hi Konstantin,

On 6/4/2017 9:14 PM, Ananyev, Konstantin wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Ashish Jain [mailto:ashish.jain@nxp.com]
>> Sent: Thursday, April 20, 2017 8:18 AM
>> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Subject: [PATCH] examples/ip_fragmentation: add fragmentation size support
>>
>> Adding support for determining fragmentation size for both
>> ipv4 and ipv6 traffic dynamically through command line.
>> It is helpful in testing to configure different fragmentation
>> sizes and validate the packets.
>>
>> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>   examples/ip_fragmentation/main.c | 89 ++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 81 insertions(+), 8 deletions(-)
>>
>> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
>> index 815b225..436755b 100644
>> --- a/examples/ip_fragmentation/main.c
>> +++ b/examples/ip_fragmentation/main.c
>> @@ -94,6 +94,16 @@
>>   #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
>>
>>   /*
>> + * Configure fragmentation size for IPv4 and IPv6 packets
>> + */
>> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
>> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
>> +#define MIN_IPV4_FRAG_SIZE 64
>> +#define MAX_IPV4_FRAG_SIZE 9600
>> +#define MIN_IPV6_FRAG_SIZE 1280
>> +#define MAX_IPV6_FRAG_SIZE 9600
>> +
>> +/*
>>    * Max number of fragments per packet expected - defined by config file.
>>    */
>>   #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
>> @@ -299,14 +309,14 @@ struct rte_lpm6_config lpm6_config = {
>>   		}
>>
>>   		/* if we don't need to do any fragmentation */
>> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
>> +		if (likely (frag_size_v4 >= m->pkt_len)) {
>>   			qconf->tx_mbufs[port_out].m_table[len] = m;
>>   			len2 = 1;
>>   		} else {
>>   			len2 = rte_ipv4_fragment_packet(m,
>>   				&qconf->tx_mbufs[port_out].m_table[len],
>>   				(uint16_t)(MBUF_TABLE_SIZE - len),
>> -				IPV4_MTU_DEFAULT,
>> +				frag_size_v4,
>>   				rxq->direct_pool, rxq->indirect_pool);
>>
>>   			/* Free input packet */
>> @@ -336,14 +346,14 @@ struct rte_lpm6_config lpm6_config = {
>>   		}
>>
>>   		/* if we don't need to do any fragmentation */
>> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
>> +		if (likely (frag_size_v6 >= m->pkt_len)) {
>>   			qconf->tx_mbufs[port_out].m_table[len] = m;
>>   			len2 = 1;
>>   		} else {
>>   			len2 = rte_ipv6_fragment_packet(m,
>>   				&qconf->tx_mbufs[port_out].m_table[len],
>>   				(uint16_t)(MBUF_TABLE_SIZE - len),
>> -				IPV6_MTU_DEFAULT,
>> +				frag_size_v6,
>>   				rxq->direct_pool, rxq->indirect_pool);
>>
>>   			/* Free input packet */
>> @@ -489,8 +499,14 @@ struct rte_lpm6_config lpm6_config = {
>>   {
>>   	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>>   	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
>> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
>> -	       prgname);
>> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
>> +	       "  --frag_size_v4=<num>:optional,IPv4 fragment size in decimal"
>> +	       ",Condition:(frag_size_v4 - 20) should be a multiple of 8,"
>> +	       " default is %d \n"
>> +	       "  --frag_size_v6=<num>:optional,IPv6 fragment size in decimal"
>> +	       ",Condition:(frag_size_v6 - 40) should be a multiple of 8,"
>> +	       " default is %d\n",
>> +	       prgname, frag_size_v4, frag_size_v6);
>>   }
>>
>>   static int
>> @@ -528,6 +544,29 @@ struct rte_lpm6_config lpm6_config = {
>>   	return n;
>>   }
>>
>> +static int
>> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
>> +		uint8_t hdr_size, uint32_t *val)
>> +{
>> +	char *end;
>> +	uint64_t v;
>> +
>> +	/* parse decimal string */
>> +	errno = 0;
>> +	v = strtoul(str, &end, 10);
>> +	if (errno != 0 || *end != '\0')
>> +		return -EINVAL;
>> +
>> +	if (v < min || v > max)
>> +		return -EINVAL;
>> +
>> +	if ((v - hdr_size) % 8)
>> +		return -EINVAL;
>> +
>> +	*val = (uint32_t)v;
>> +	return 0;
>> +}
>> +
>>   /* Parse the argument given in the command line of the application */
>>   static int
>>   parse_args(int argc, char **argv)
>> @@ -537,6 +576,8 @@ struct rte_lpm6_config lpm6_config = {
>>   	int option_index;
>>   	char *prgname = argv[0];
>>   	static struct option lgopts[] = {
>> +		{"frag_size_v4", 1, 0, 0},
>> +		{"frag_size_v6", 1, 0, 0},
>>   		{NULL, 0, 0, 0}
>>   	};
>>
>> @@ -568,8 +609,40 @@ struct rte_lpm6_config lpm6_config = {
>>
>>   		/* long options */
>>   		case 0:
>> -			print_usage(prgname);
>> -			return -1;
>> +			if (!strncmp(lgopts[option_index].name,
>> +				"frag_size_v4", 12)) {
> 
> 
> Why just not strcmp() here?
strncmp() here is adhering to its similar use in other applications like 
ip_reassemble, l3fwd etc.
> 
>> +				ret = parse_frag_size(optarg,
>> +						MIN_IPV4_FRAG_SIZE,
>> +						MAX_IPV4_FRAG_SIZE,
>> +						sizeof(struct ipv4_hdr),
>> +						&frag_size_v4);
>> +				if (ret) {
>> +					printf("invalid value: \"%s\" for "
>> +						"parameter %s\n",
>> +						optarg,
>> +						lgopts[option_index].name);
>> +					print_usage(prgname);
>> +					return ret;
>> +				}
>> +			}
>> +			if (!strncmp(lgopts[option_index].name,
>> +				"frag_size_v6", 12)) {
>> +				ret = parse_frag_size(optarg,
>> +						MIN_IPV6_FRAG_SIZE,
>> +						MAX_IPV6_FRAG_SIZE,
>> +						sizeof(struct ipv6_hdr),
>> +						&frag_size_v6);
>> +				if (ret) {
>> +					printf("invalid value: \"%s\" for "
>> +						"parameter %s\n",
>> +						optarg,
>> +						lgopts[option_index].name);
>> +					print_usage(prgname);
>> +					return ret;
>> +				}
>> +			}
>> +
>> +			break;
> 
> Hmm, why do we need that break here?
> Konstantin
'break' is needed here to exit from the switch case

Ashish
> 
> 
>>
>>   		default:
>>   			print_usage(prgname);
>> --
>> 1.9.1
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] examples/ip_fragmentation: add fragmentation size support
  2017-06-14  9:19   ` Jain Ashish-B46179
@ 2017-06-14 10:10     ` Ananyev, Konstantin
  0 siblings, 0 replies; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-06-14 10:10 UTC (permalink / raw)
  To: Jain Ashish-B46179, dev



> -----Original Message-----
> From: Jain Ashish-B46179 [mailto:ashish.jain@nxp.com]
> Sent: Wednesday, June 14, 2017 10:20 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] examples/ip_fragmentation: add fragmentation size support
> 
> Hi Konstantin,
> 
> On 6/4/2017 9:14 PM, Ananyev, Konstantin wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Ashish Jain [mailto:ashish.jain@nxp.com]
> >> Sent: Thursday, April 20, 2017 8:18 AM
> >> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> >> Subject: [PATCH] examples/ip_fragmentation: add fragmentation size support
> >>
> >> Adding support for determining fragmentation size for both
> >> ipv4 and ipv6 traffic dynamically through command line.
> >> It is helpful in testing to configure different fragmentation
> >> sizes and validate the packets.
> >>
> >> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >>   examples/ip_fragmentation/main.c | 89 ++++++++++++++++++++++++++++++++++++----
> >>   1 file changed, 81 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> >> index 815b225..436755b 100644
> >> --- a/examples/ip_fragmentation/main.c
> >> +++ b/examples/ip_fragmentation/main.c
> >> @@ -94,6 +94,16 @@
> >>   #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
> >>
> >>   /*
> >> + * Configure fragmentation size for IPv4 and IPv6 packets
> >> + */
> >> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
> >> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
> >> +#define MIN_IPV4_FRAG_SIZE 64
> >> +#define MAX_IPV4_FRAG_SIZE 9600
> >> +#define MIN_IPV6_FRAG_SIZE 1280
> >> +#define MAX_IPV6_FRAG_SIZE 9600
> >> +
> >> +/*
> >>    * Max number of fragments per packet expected - defined by config file.
> >>    */
> >>   #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
> >> @@ -299,14 +309,14 @@ struct rte_lpm6_config lpm6_config = {
> >>   		}
> >>
> >>   		/* if we don't need to do any fragmentation */
> >> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
> >> +		if (likely (frag_size_v4 >= m->pkt_len)) {
> >>   			qconf->tx_mbufs[port_out].m_table[len] = m;
> >>   			len2 = 1;
> >>   		} else {
> >>   			len2 = rte_ipv4_fragment_packet(m,
> >>   				&qconf->tx_mbufs[port_out].m_table[len],
> >>   				(uint16_t)(MBUF_TABLE_SIZE - len),
> >> -				IPV4_MTU_DEFAULT,
> >> +				frag_size_v4,
> >>   				rxq->direct_pool, rxq->indirect_pool);
> >>
> >>   			/* Free input packet */
> >> @@ -336,14 +346,14 @@ struct rte_lpm6_config lpm6_config = {
> >>   		}
> >>
> >>   		/* if we don't need to do any fragmentation */
> >> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
> >> +		if (likely (frag_size_v6 >= m->pkt_len)) {
> >>   			qconf->tx_mbufs[port_out].m_table[len] = m;
> >>   			len2 = 1;
> >>   		} else {
> >>   			len2 = rte_ipv6_fragment_packet(m,
> >>   				&qconf->tx_mbufs[port_out].m_table[len],
> >>   				(uint16_t)(MBUF_TABLE_SIZE - len),
> >> -				IPV6_MTU_DEFAULT,
> >> +				frag_size_v6,
> >>   				rxq->direct_pool, rxq->indirect_pool);
> >>
> >>   			/* Free input packet */
> >> @@ -489,8 +499,14 @@ struct rte_lpm6_config lpm6_config = {
> >>   {
> >>   	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
> >>   	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> >> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
> >> -	       prgname);
> >> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> >> +	       "  --frag_size_v4=<num>:optional,IPv4 fragment size in decimal"
> >> +	       ",Condition:(frag_size_v4 - 20) should be a multiple of 8,"
> >> +	       " default is %d \n"
> >> +	       "  --frag_size_v6=<num>:optional,IPv6 fragment size in decimal"
> >> +	       ",Condition:(frag_size_v6 - 40) should be a multiple of 8,"
> >> +	       " default is %d\n",
> >> +	       prgname, frag_size_v4, frag_size_v6);
> >>   }
> >>
> >>   static int
> >> @@ -528,6 +544,29 @@ struct rte_lpm6_config lpm6_config = {
> >>   	return n;
> >>   }
> >>
> >> +static int
> >> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
> >> +		uint8_t hdr_size, uint32_t *val)
> >> +{
> >> +	char *end;
> >> +	uint64_t v;
> >> +
> >> +	/* parse decimal string */
> >> +	errno = 0;
> >> +	v = strtoul(str, &end, 10);
> >> +	if (errno != 0 || *end != '\0')
> >> +		return -EINVAL;
> >> +
> >> +	if (v < min || v > max)
> >> +		return -EINVAL;
> >> +
> >> +	if ((v - hdr_size) % 8)
> >> +		return -EINVAL;
> >> +
> >> +	*val = (uint32_t)v;
> >> +	return 0;
> >> +}
> >> +
> >>   /* Parse the argument given in the command line of the application */
> >>   static int
> >>   parse_args(int argc, char **argv)
> >> @@ -537,6 +576,8 @@ struct rte_lpm6_config lpm6_config = {
> >>   	int option_index;
> >>   	char *prgname = argv[0];
> >>   	static struct option lgopts[] = {
> >> +		{"frag_size_v4", 1, 0, 0},
> >> +		{"frag_size_v6", 1, 0, 0},
> >>   		{NULL, 0, 0, 0}
> >>   	};
> >>
> >> @@ -568,8 +609,40 @@ struct rte_lpm6_config lpm6_config = {
> >>
> >>   		/* long options */
> >>   		case 0:
> >> -			print_usage(prgname);
> >> -			return -1;
> >> +			if (!strncmp(lgopts[option_index].name,
> >> +				"frag_size_v4", 12)) {
> >
> >
> > Why just not strcmp() here?
> strncmp() here is adhering to its similar use in other applications like
> ip_reassemble, l3fwd etc.

It means that "frag_size_v4-blah" would also pass this test.
BTW, l3fwd doesn't use strncmp() here - instead it associates long option with short one.
I suggest you do the same.

> >
> >> +				ret = parse_frag_size(optarg,
> >> +						MIN_IPV4_FRAG_SIZE,
> >> +						MAX_IPV4_FRAG_SIZE,
> >> +						sizeof(struct ipv4_hdr),
> >> +						&frag_size_v4);
> >> +				if (ret) {
> >> +					printf("invalid value: \"%s\" for "
> >> +						"parameter %s\n",
> >> +						optarg,
> >> +						lgopts[option_index].name);
> >> +					print_usage(prgname);
> >> +					return ret;
> >> +				}
> >> +			}
> >> +			if (!strncmp(lgopts[option_index].name,
> >> +				"frag_size_v6", 12)) {
> >> +				ret = parse_frag_size(optarg,
> >> +						MIN_IPV6_FRAG_SIZE,
> >> +						MAX_IPV6_FRAG_SIZE,
> >> +						sizeof(struct ipv6_hdr),
> >> +						&frag_size_v6);
> >> +				if (ret) {
> >> +					printf("invalid value: \"%s\" for "
> >> +						"parameter %s\n",
> >> +						optarg,
> >> +						lgopts[option_index].name);
> >> +					print_usage(prgname);
> >> +					return ret;
> >> +				}
> >> +			}
> >> +
> >> +			break;
> >
> > Hmm, why do we need that break here?
> > Konstantin
> 'break' is needed here to exit from the switch case

Ok, so it means it is allowed to pass any other long options here?
Konstantin


> 
> Ashish
> >
> >
> >>
> >>   		default:
> >>   			print_usage(prgname);
> >> --
> >> 1.9.1
> >
> >
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] example/ip_fragmentation: add fragmentation size support
  2017-04-20  7:18 [PATCH] examples/ip_fragmentation: add fragmentation size support Ashish Jain
  2017-05-24  7:28 ` Ashish Jain
  2017-06-04 15:44 ` Ananyev, Konstantin
@ 2017-06-30 12:18 ` Ashish Jain
  2017-06-30 12:34   ` Ananyev, Konstantin
  2 siblings, 1 reply; 9+ messages in thread
From: Ashish Jain @ 2017-06-30 12:18 UTC (permalink / raw)
  To: konstantin.ananyev, dev; +Cc: Ashish Jain, Hemant Agrawal

Adding support for determining fragmentation size for both
ipv4 and ipv6 traffic dynamically through command line.
It is helpful in testing to configure different fragmentation
sizes and validate the packets.

Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
Changes in v2:
* used strncmp while associating long options with short ones
* added detailed usage for new added params

 examples/ip_fragmentation/main.c | 96 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 88 insertions(+), 8 deletions(-)

diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
index 2f45264..c0b36cd 100644
--- a/examples/ip_fragmentation/main.c
+++ b/examples/ip_fragmentation/main.c
@@ -95,6 +95,16 @@
 #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
 
 /*
+ * Configure fragmentation size for IPv4 and IPv6 packets
+ */
+static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
+static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
+#define MIN_IPV4_FRAG_SIZE 64
+#define MAX_IPV4_FRAG_SIZE 9600
+#define MIN_IPV6_FRAG_SIZE 1280
+#define MAX_IPV6_FRAG_SIZE 9600
+
+/*
  * Max number of fragments per packet expected - defined by config file.
  */
 #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
@@ -139,6 +149,9 @@ static struct ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
 
 #define IPV6_ADDR_LEN 16
 
+#define CMD_LINE_OPT_IPV4_FRAG_SIZE "frag_size_v4"
+#define CMD_LINE_OPT_IPV6_FRAG_SIZE "frag_size_v6"
+
 /* mask of enabled ports */
 static int enabled_port_mask = 0;
 
@@ -300,14 +313,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
 		}
 
 		/* if we don't need to do any fragmentation */
-		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
+		if (likely(frag_size_v4 >= m->pkt_len)) {
 			qconf->tx_mbufs[port_out].m_table[len] = m;
 			len2 = 1;
 		} else {
 			len2 = rte_ipv4_fragment_packet(m,
 				&qconf->tx_mbufs[port_out].m_table[len],
 				(uint16_t)(MBUF_TABLE_SIZE - len),
-				IPV4_MTU_DEFAULT,
+				frag_size_v4,
 				rxq->direct_pool, rxq->indirect_pool);
 
 			/* Free input packet */
@@ -336,14 +349,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
 		}
 
 		/* if we don't need to do any fragmentation */
-		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
+		if (likely(frag_size_v6 >= m->pkt_len)) {
 			qconf->tx_mbufs[port_out].m_table[len] = m;
 			len2 = 1;
 		} else {
 			len2 = rte_ipv6_fragment_packet(m,
 				&qconf->tx_mbufs[port_out].m_table[len],
 				(uint16_t)(MBUF_TABLE_SIZE - len),
-				IPV6_MTU_DEFAULT,
+				frag_size_v6,
 				rxq->direct_pool, rxq->indirect_pool);
 
 			/* Free input packet */
@@ -489,8 +502,16 @@ print_usage(const char *prgname)
 {
 	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
 	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
-	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
-	       prgname);
+	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
+	       "  --frag_size_v4=<num>: optional,  IPv4 fragment size in decimal,\n"
+	       "\t Condition:(frag_size_v4 - 20) should be a multiple of 8\n"
+	       "\t Min value: %d , Max value: %d, default is %d \n"
+	       "  --frag_size_v6=<num>: optional,  IPv6 fragment size in decimal,\n"
+	       "\t Condition:(frag_size_v6 - 40) should be a multiple of 8\n"
+	       "\t Min value: %d , Max value: %d, default is %d \n",
+	       prgname, MIN_IPV4_FRAG_SIZE, MAX_IPV4_FRAG_SIZE,
+	       IPV4_MTU_DEFAULT, MIN_IPV6_FRAG_SIZE, MAX_IPV6_FRAG_SIZE,
+	       IPV6_MTU_DEFAULT);
 }
 
 static int
@@ -528,6 +549,29 @@ parse_nqueue(const char *q_arg)
 	return n;
 }
 
+static int
+parse_frag_size(const char *str, uint32_t min, uint32_t max,
+		uint8_t hdr_size, uint32_t *val)
+{
+	char *end;
+	uint64_t v;
+
+	/* parse decimal string */
+	errno = 0;
+	v = strtoul(str, &end, 10);
+	if (errno != 0 || *end != '\0')
+		return -EINVAL;
+
+	if (v < min || v > max)
+		return -EINVAL;
+
+	if ((v - hdr_size) % 8)
+		return -EINVAL;
+
+	*val = (uint32_t)v;
+	return 0;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 parse_args(int argc, char **argv)
@@ -537,6 +581,8 @@ parse_args(int argc, char **argv)
 	int option_index;
 	char *prgname = argv[0];
 	static struct option lgopts[] = {
+		{"frag_size_v4", 1, 0, 0},
+		{"frag_size_v6", 1, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -568,8 +614,42 @@ parse_args(int argc, char **argv)
 
 		/* long options */
 		case 0:
-			print_usage(prgname);
-			return -1;
+			if (!strncmp(lgopts[option_index].name,
+				CMD_LINE_OPT_IPV4_FRAG_SIZE,
+				sizeof(CMD_LINE_OPT_IPV4_FRAG_SIZE))) {
+				ret = parse_frag_size(optarg,
+						MIN_IPV4_FRAG_SIZE,
+						MAX_IPV4_FRAG_SIZE,
+						sizeof(struct ipv4_hdr),
+						&frag_size_v4);
+				if (ret) {
+					printf("invalid value: \"%s\" for "
+						"parameter %s\n",
+						optarg,
+						lgopts[option_index].name);
+					print_usage(prgname);
+					return ret;
+				}
+			}
+			if (!strncmp(lgopts[option_index].name,
+				CMD_LINE_OPT_IPV6_FRAG_SIZE,
+				sizeof(CMD_LINE_OPT_IPV6_FRAG_SIZE))) {
+				ret = parse_frag_size(optarg,
+						MIN_IPV6_FRAG_SIZE,
+						MAX_IPV6_FRAG_SIZE,
+						sizeof(struct ipv6_hdr),
+						&frag_size_v6);
+				if (ret) {
+					printf("invalid value: \"%s\" for "
+						"parameter %s\n",
+						optarg,
+						lgopts[option_index].name);
+					print_usage(prgname);
+					return ret;
+				}
+			}
+
+			break;
 
 		default:
 			print_usage(prgname);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] example/ip_fragmentation: add fragmentation size support
  2017-06-30 12:18 ` [PATCH v2] example/ip_fragmentation: " Ashish Jain
@ 2017-06-30 12:34   ` Ananyev, Konstantin
  2017-06-30 12:50     ` Jain Ashish-B46179
  0 siblings, 1 reply; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-06-30 12:34 UTC (permalink / raw)
  To: Ashish Jain, dev; +Cc: Hemant Agrawal



> -----Original Message-----
> From: Ashish Jain [mailto:ashish.jain@nxp.com]
> Sent: Friday, June 30, 2017 1:18 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: Ashish Jain <ashish.jain@nxp.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: [PATCH v2] example/ip_fragmentation: add fragmentation size support
> 
> Adding support for determining fragmentation size for both
> ipv4 and ipv6 traffic dynamically through command line.
> It is helpful in testing to configure different fragmentation
> sizes and validate the packets.
> 
> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> Changes in v2:
> * used strncmp while associating long options with short ones
> * added detailed usage for new added params
> 
>  examples/ip_fragmentation/main.c | 96 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 88 insertions(+), 8 deletions(-)
> 
> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> index 2f45264..c0b36cd 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -95,6 +95,16 @@
>  #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
> 
>  /*
> + * Configure fragmentation size for IPv4 and IPv6 packets
> + */
> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
> +#define MIN_IPV4_FRAG_SIZE 64
> +#define MAX_IPV4_FRAG_SIZE 9600
> +#define MIN_IPV6_FRAG_SIZE 1280
> +#define MAX_IPV6_FRAG_SIZE 9600
> +
> +/*
>   * Max number of fragments per packet expected - defined by config file.
>   */
>  #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
> @@ -139,6 +149,9 @@ static struct ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
> 
>  #define IPV6_ADDR_LEN 16
> 
> +#define CMD_LINE_OPT_IPV4_FRAG_SIZE "frag_size_v4"
> +#define CMD_LINE_OPT_IPV6_FRAG_SIZE "frag_size_v6"
> +
>  /* mask of enabled ports */
>  static int enabled_port_mask = 0;
> 
> @@ -300,14 +313,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
>  		}
> 
>  		/* if we don't need to do any fragmentation */
> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
> +		if (likely(frag_size_v4 >= m->pkt_len)) {
>  			qconf->tx_mbufs[port_out].m_table[len] = m;
>  			len2 = 1;
>  		} else {
>  			len2 = rte_ipv4_fragment_packet(m,
>  				&qconf->tx_mbufs[port_out].m_table[len],
>  				(uint16_t)(MBUF_TABLE_SIZE - len),
> -				IPV4_MTU_DEFAULT,
> +				frag_size_v4,
>  				rxq->direct_pool, rxq->indirect_pool);
> 
>  			/* Free input packet */
> @@ -336,14 +349,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
>  		}
> 
>  		/* if we don't need to do any fragmentation */
> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
> +		if (likely(frag_size_v6 >= m->pkt_len)) {
>  			qconf->tx_mbufs[port_out].m_table[len] = m;
>  			len2 = 1;
>  		} else {
>  			len2 = rte_ipv6_fragment_packet(m,
>  				&qconf->tx_mbufs[port_out].m_table[len],
>  				(uint16_t)(MBUF_TABLE_SIZE - len),
> -				IPV6_MTU_DEFAULT,
> +				frag_size_v6,
>  				rxq->direct_pool, rxq->indirect_pool);
> 
>  			/* Free input packet */
> @@ -489,8 +502,16 @@ print_usage(const char *prgname)
>  {
>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>  	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
> -	       prgname);
> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> +	       "  --frag_size_v4=<num>: optional,  IPv4 fragment size in decimal,\n"
> +	       "\t Condition:(frag_size_v4 - 20) should be a multiple of 8\n"
> +	       "\t Min value: %d , Max value: %d, default is %d \n"
> +	       "  --frag_size_v6=<num>: optional,  IPv6 fragment size in decimal,\n"
> +	       "\t Condition:(frag_size_v6 - 40) should be a multiple of 8\n"
> +	       "\t Min value: %d , Max value: %d, default is %d \n",
> +	       prgname, MIN_IPV4_FRAG_SIZE, MAX_IPV4_FRAG_SIZE,
> +	       IPV4_MTU_DEFAULT, MIN_IPV6_FRAG_SIZE, MAX_IPV6_FRAG_SIZE,
> +	       IPV6_MTU_DEFAULT);
>  }
> 
>  static int
> @@ -528,6 +549,29 @@ parse_nqueue(const char *q_arg)
>  	return n;
>  }
> 
> +static int
> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
> +		uint8_t hdr_size, uint32_t *val)
> +{
> +	char *end;
> +	uint64_t v;
> +
> +	/* parse decimal string */
> +	errno = 0;
> +	v = strtoul(str, &end, 10);
> +	if (errno != 0 || *end != '\0')
> +		return -EINVAL;
> +
> +	if (v < min || v > max)
> +		return -EINVAL;
> +
> +	if ((v - hdr_size) % 8)
> +		return -EINVAL;
> +
> +	*val = (uint32_t)v;
> +	return 0;
> +}
> +
>  /* Parse the argument given in the command line of the application */
>  static int
>  parse_args(int argc, char **argv)
> @@ -537,6 +581,8 @@ parse_args(int argc, char **argv)
>  	int option_index;
>  	char *prgname = argv[0];
>  	static struct option lgopts[] = {
> +		{"frag_size_v4", 1, 0, 0},
> +		{"frag_size_v6", 1, 0, 0},
>  		{NULL, 0, 0, 0}
>  	};
> 
> @@ -568,8 +614,42 @@ parse_args(int argc, char **argv)
> 
>  		/* long options */
>  		case 0:
> -			print_usage(prgname);
> -			return -1;
> +			if (!strncmp(lgopts[option_index].name,
> +				CMD_LINE_OPT_IPV4_FRAG_SIZE,
> +				sizeof(CMD_LINE_OPT_IPV4_FRAG_SIZE))) {
> +				ret = parse_frag_size(optarg,
> +						MIN_IPV4_FRAG_SIZE,
> +						MAX_IPV4_FRAG_SIZE,
> +						sizeof(struct ipv4_hdr),
> +						&frag_size_v4);
> +				if (ret) {
> +					printf("invalid value: \"%s\" for "
> +						"parameter %s\n",
> +						optarg,
> +						lgopts[option_index].name);
> +					print_usage(prgname);
> +					return ret;
> +				}
> +			}
> +			if (!strncmp(lgopts[option_index].name,
> +				CMD_LINE_OPT_IPV6_FRAG_SIZE,
> +				sizeof(CMD_LINE_OPT_IPV6_FRAG_SIZE))) {
> +				ret = parse_frag_size(optarg,
> +						MIN_IPV6_FRAG_SIZE,
> +						MAX_IPV6_FRAG_SIZE,
> +						sizeof(struct ipv6_hdr),
> +						&frag_size_v6);
> +				if (ret) {
> +					printf("invalid value: \"%s\" for "
> +						"parameter %s\n",
> +						optarg,
> +						lgopts[option_index].name);
> +					print_usage(prgname);
> +					return ret;
> +				}
> +			}
> +
> +			break;

Seems you ignored my comments for v1?
Konstantin

> 
>  		default:
>  			print_usage(prgname);
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] example/ip_fragmentation: add fragmentation size support
  2017-06-30 12:34   ` Ananyev, Konstantin
@ 2017-06-30 12:50     ` Jain Ashish-B46179
  2017-06-30 12:56       ` Ananyev, Konstantin
  0 siblings, 1 reply; 9+ messages in thread
From: Jain Ashish-B46179 @ 2017-06-30 12:50 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev; +Cc: Hemant Agrawal

On 6/30/2017 6:04 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Ashish Jain [mailto:ashish.jain@nxp.com]
>> Sent: Friday, June 30, 2017 1:18 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
>> Cc: Ashish Jain <ashish.jain@nxp.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
>> Subject: [PATCH v2] example/ip_fragmentation: add fragmentation size support
>>
>> Adding support for determining fragmentation size for both
>> ipv4 and ipv6 traffic dynamically through command line.
>> It is helpful in testing to configure different fragmentation
>> sizes and validate the packets.
>>
>> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> Changes in v2:
>> * used strncmp while associating long options with short ones
>> * added detailed usage for new added params
>>
>>   examples/ip_fragmentation/main.c | 96 ++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 88 insertions(+), 8 deletions(-)
>>
>> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
>> index 2f45264..c0b36cd 100644
>> --- a/examples/ip_fragmentation/main.c
>> +++ b/examples/ip_fragmentation/main.c
>> @@ -95,6 +95,16 @@
>>   #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
>>
>>   /*
>> + * Configure fragmentation size for IPv4 and IPv6 packets
>> + */
>> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
>> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
>> +#define MIN_IPV4_FRAG_SIZE 64
>> +#define MAX_IPV4_FRAG_SIZE 9600
>> +#define MIN_IPV6_FRAG_SIZE 1280
>> +#define MAX_IPV6_FRAG_SIZE 9600
>> +
>> +/*
>>    * Max number of fragments per packet expected - defined by config file.
>>    */
>>   #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
>> @@ -139,6 +149,9 @@ static struct ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
>>
>>   #define IPV6_ADDR_LEN 16
>>
>> +#define CMD_LINE_OPT_IPV4_FRAG_SIZE "frag_size_v4"
>> +#define CMD_LINE_OPT_IPV6_FRAG_SIZE "frag_size_v6"
>> +
>>   /* mask of enabled ports */
>>   static int enabled_port_mask = 0;
>>
>> @@ -300,14 +313,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
>>   		}
>>
>>   		/* if we don't need to do any fragmentation */
>> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
>> +		if (likely(frag_size_v4 >= m->pkt_len)) {
>>   			qconf->tx_mbufs[port_out].m_table[len] = m;
>>   			len2 = 1;
>>   		} else {
>>   			len2 = rte_ipv4_fragment_packet(m,
>>   				&qconf->tx_mbufs[port_out].m_table[len],
>>   				(uint16_t)(MBUF_TABLE_SIZE - len),
>> -				IPV4_MTU_DEFAULT,
>> +				frag_size_v4,
>>   				rxq->direct_pool, rxq->indirect_pool);
>>
>>   			/* Free input packet */
>> @@ -336,14 +349,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
>>   		}
>>
>>   		/* if we don't need to do any fragmentation */
>> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
>> +		if (likely(frag_size_v6 >= m->pkt_len)) {
>>   			qconf->tx_mbufs[port_out].m_table[len] = m;
>>   			len2 = 1;
>>   		} else {
>>   			len2 = rte_ipv6_fragment_packet(m,
>>   				&qconf->tx_mbufs[port_out].m_table[len],
>>   				(uint16_t)(MBUF_TABLE_SIZE - len),
>> -				IPV6_MTU_DEFAULT,
>> +				frag_size_v6,
>>   				rxq->direct_pool, rxq->indirect_pool);
>>
>>   			/* Free input packet */
>> @@ -489,8 +502,16 @@ print_usage(const char *prgname)
>>   {
>>   	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>>   	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
>> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
>> -	       prgname);
>> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
>> +	       "  --frag_size_v4=<num>: optional,  IPv4 fragment size in decimal,\n"
>> +	       "\t Condition:(frag_size_v4 - 20) should be a multiple of 8\n"
>> +	       "\t Min value: %d , Max value: %d, default is %d \n"
>> +	       "  --frag_size_v6=<num>: optional,  IPv6 fragment size in decimal,\n"
>> +	       "\t Condition:(frag_size_v6 - 40) should be a multiple of 8\n"
>> +	       "\t Min value: %d , Max value: %d, default is %d \n",
>> +	       prgname, MIN_IPV4_FRAG_SIZE, MAX_IPV4_FRAG_SIZE,
>> +	       IPV4_MTU_DEFAULT, MIN_IPV6_FRAG_SIZE, MAX_IPV6_FRAG_SIZE,
>> +	       IPV6_MTU_DEFAULT);
>>   }
>>
>>   static int
>> @@ -528,6 +549,29 @@ parse_nqueue(const char *q_arg)
>>   	return n;
>>   }
>>
>> +static int
>> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
>> +		uint8_t hdr_size, uint32_t *val)
>> +{
>> +	char *end;
>> +	uint64_t v;
>> +
>> +	/* parse decimal string */
>> +	errno = 0;
>> +	v = strtoul(str, &end, 10);
>> +	if (errno != 0 || *end != '\0')
>> +		return -EINVAL;
>> +
>> +	if (v < min || v > max)
>> +		return -EINVAL;
>> +
>> +	if ((v - hdr_size) % 8)
>> +		return -EINVAL;
>> +
>> +	*val = (uint32_t)v;
>> +	return 0;
>> +}
>> +
>>   /* Parse the argument given in the command line of the application */
>>   static int
>>   parse_args(int argc, char **argv)
>> @@ -537,6 +581,8 @@ parse_args(int argc, char **argv)
>>   	int option_index;
>>   	char *prgname = argv[0];
>>   	static struct option lgopts[] = {
>> +		{"frag_size_v4", 1, 0, 0},
>> +		{"frag_size_v6", 1, 0, 0},
>>   		{NULL, 0, 0, 0}
>>   	};
>>
>> @@ -568,8 +614,42 @@ parse_args(int argc, char **argv)
>>
>>   		/* long options */
>>   		case 0:
>> -			print_usage(prgname);
>> -			return -1;
>> +			if (!strncmp(lgopts[option_index].name,
>> +				CMD_LINE_OPT_IPV4_FRAG_SIZE,
>> +				sizeof(CMD_LINE_OPT_IPV4_FRAG_SIZE))) {
>> +				ret = parse_frag_size(optarg,
>> +						MIN_IPV4_FRAG_SIZE,
>> +						MAX_IPV4_FRAG_SIZE,
>> +						sizeof(struct ipv4_hdr),
>> +						&frag_size_v4);
>> +				if (ret) {
>> +					printf("invalid value: \"%s\" for "
>> +						"parameter %s\n",
>> +						optarg,
>> +						lgopts[option_index].name);
>> +					print_usage(prgname);
>> +					return ret;
>> +				}
>> +			}
>> +			if (!strncmp(lgopts[option_index].name,
>> +				CMD_LINE_OPT_IPV6_FRAG_SIZE,
>> +				sizeof(CMD_LINE_OPT_IPV6_FRAG_SIZE))) {
>> +				ret = parse_frag_size(optarg,
>> +						MIN_IPV6_FRAG_SIZE,
>> +						MAX_IPV6_FRAG_SIZE,
>> +						sizeof(struct ipv6_hdr),
>> +						&frag_size_v6);
>> +				if (ret) {
>> +					printf("invalid value: \"%s\" for "
>> +						"parameter %s\n",
>> +						optarg,
>> +						lgopts[option_index].name);
>> +					print_usage(prgname);
>> +					return ret;
>> +				}
>> +			}
>> +
>> +			break;
> 
> Seems you ignored my comments for v1?
> Konstantin
Sorry, I forgot to reply to this comment.
'break' here is needed to avoid fall through to the default case.
If any other long option is passed in command line then application 
should exit.
Ashish
> 
>>
>>   		default:
>>   			print_usage(prgname);
>> --
>> 2.7.4
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] example/ip_fragmentation: add fragmentation size support
  2017-06-30 12:50     ` Jain Ashish-B46179
@ 2017-06-30 12:56       ` Ananyev, Konstantin
  0 siblings, 0 replies; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-06-30 12:56 UTC (permalink / raw)
  To: Jain Ashish-B46179, dev; +Cc: Hemant Agrawal



> -----Original Message-----
> From: Jain Ashish-B46179 [mailto:ashish.jain@nxp.com]
> Sent: Friday, June 30, 2017 1:50 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: Re: [PATCH v2] example/ip_fragmentation: add fragmentation size support
> 
> On 6/30/2017 6:04 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ashish Jain [mailto:ashish.jain@nxp.com]
> >> Sent: Friday, June 30, 2017 1:18 PM
> >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> >> Cc: Ashish Jain <ashish.jain@nxp.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> >> Subject: [PATCH v2] example/ip_fragmentation: add fragmentation size support
> >>
> >> Adding support for determining fragmentation size for both
> >> ipv4 and ipv6 traffic dynamically through command line.
> >> It is helpful in testing to configure different fragmentation
> >> sizes and validate the packets.
> >>
> >> Signed-off-by: Ashish Jain <ashish.jain@nxp.com>
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >> Changes in v2:
> >> * used strncmp while associating long options with short ones
> >> * added detailed usage for new added params
> >>
> >>   examples/ip_fragmentation/main.c | 96 ++++++++++++++++++++++++++++++++++++----
> >>   1 file changed, 88 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> >> index 2f45264..c0b36cd 100644
> >> --- a/examples/ip_fragmentation/main.c
> >> +++ b/examples/ip_fragmentation/main.c
> >> @@ -95,6 +95,16 @@
> >>   #define	IPV6_DEFAULT_PAYLOAD	(IPV6_MTU_DEFAULT - sizeof(struct ipv6_hdr))
> >>
> >>   /*
> >> + * Configure fragmentation size for IPv4 and IPv6 packets
> >> + */
> >> +static uint32_t frag_size_v4 = IPV4_MTU_DEFAULT;
> >> +static uint32_t frag_size_v6 = IPV6_MTU_DEFAULT;
> >> +#define MIN_IPV4_FRAG_SIZE 64
> >> +#define MAX_IPV4_FRAG_SIZE 9600
> >> +#define MIN_IPV6_FRAG_SIZE 1280
> >> +#define MAX_IPV6_FRAG_SIZE 9600
> >> +
> >> +/*
> >>    * Max number of fragments per packet expected - defined by config file.
> >>    */
> >>   #define	MAX_PACKET_FRAG RTE_LIBRTE_IP_FRAG_MAX_FRAG
> >> @@ -139,6 +149,9 @@ static struct ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
> >>
> >>   #define IPV6_ADDR_LEN 16
> >>
> >> +#define CMD_LINE_OPT_IPV4_FRAG_SIZE "frag_size_v4"
> >> +#define CMD_LINE_OPT_IPV6_FRAG_SIZE "frag_size_v6"
> >> +
> >>   /* mask of enabled ports */
> >>   static int enabled_port_mask = 0;
> >>
> >> @@ -300,14 +313,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
> >>   		}
> >>
> >>   		/* if we don't need to do any fragmentation */
> >> -		if (likely (IPV4_MTU_DEFAULT >= m->pkt_len)) {
> >> +		if (likely(frag_size_v4 >= m->pkt_len)) {
> >>   			qconf->tx_mbufs[port_out].m_table[len] = m;
> >>   			len2 = 1;
> >>   		} else {
> >>   			len2 = rte_ipv4_fragment_packet(m,
> >>   				&qconf->tx_mbufs[port_out].m_table[len],
> >>   				(uint16_t)(MBUF_TABLE_SIZE - len),
> >> -				IPV4_MTU_DEFAULT,
> >> +				frag_size_v4,
> >>   				rxq->direct_pool, rxq->indirect_pool);
> >>
> >>   			/* Free input packet */
> >> @@ -336,14 +349,14 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
> >>   		}
> >>
> >>   		/* if we don't need to do any fragmentation */
> >> -		if (likely (IPV6_MTU_DEFAULT >= m->pkt_len)) {
> >> +		if (likely(frag_size_v6 >= m->pkt_len)) {
> >>   			qconf->tx_mbufs[port_out].m_table[len] = m;
> >>   			len2 = 1;
> >>   		} else {
> >>   			len2 = rte_ipv6_fragment_packet(m,
> >>   				&qconf->tx_mbufs[port_out].m_table[len],
> >>   				(uint16_t)(MBUF_TABLE_SIZE - len),
> >> -				IPV6_MTU_DEFAULT,
> >> +				frag_size_v6,
> >>   				rxq->direct_pool, rxq->indirect_pool);
> >>
> >>   			/* Free input packet */
> >> @@ -489,8 +502,16 @@ print_usage(const char *prgname)
> >>   {
> >>   	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
> >>   	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> >> -	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n",
> >> -	       prgname);
> >> +	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> >> +	       "  --frag_size_v4=<num>: optional,  IPv4 fragment size in decimal,\n"
> >> +	       "\t Condition:(frag_size_v4 - 20) should be a multiple of 8\n"
> >> +	       "\t Min value: %d , Max value: %d, default is %d \n"
> >> +	       "  --frag_size_v6=<num>: optional,  IPv6 fragment size in decimal,\n"
> >> +	       "\t Condition:(frag_size_v6 - 40) should be a multiple of 8\n"
> >> +	       "\t Min value: %d , Max value: %d, default is %d \n",
> >> +	       prgname, MIN_IPV4_FRAG_SIZE, MAX_IPV4_FRAG_SIZE,
> >> +	       IPV4_MTU_DEFAULT, MIN_IPV6_FRAG_SIZE, MAX_IPV6_FRAG_SIZE,
> >> +	       IPV6_MTU_DEFAULT);
> >>   }
> >>
> >>   static int
> >> @@ -528,6 +549,29 @@ parse_nqueue(const char *q_arg)
> >>   	return n;
> >>   }
> >>
> >> +static int
> >> +parse_frag_size(const char *str, uint32_t min, uint32_t max,
> >> +		uint8_t hdr_size, uint32_t *val)
> >> +{
> >> +	char *end;
> >> +	uint64_t v;
> >> +
> >> +	/* parse decimal string */
> >> +	errno = 0;
> >> +	v = strtoul(str, &end, 10);
> >> +	if (errno != 0 || *end != '\0')
> >> +		return -EINVAL;
> >> +
> >> +	if (v < min || v > max)
> >> +		return -EINVAL;
> >> +
> >> +	if ((v - hdr_size) % 8)
> >> +		return -EINVAL;
> >> +
> >> +	*val = (uint32_t)v;
> >> +	return 0;
> >> +}
> >> +
> >>   /* Parse the argument given in the command line of the application */
> >>   static int
> >>   parse_args(int argc, char **argv)
> >> @@ -537,6 +581,8 @@ parse_args(int argc, char **argv)
> >>   	int option_index;
> >>   	char *prgname = argv[0];
> >>   	static struct option lgopts[] = {
> >> +		{"frag_size_v4", 1, 0, 0},
> >> +		{"frag_size_v6", 1, 0, 0},
> >>   		{NULL, 0, 0, 0}
> >>   	};
> >>
> >> @@ -568,8 +614,42 @@ parse_args(int argc, char **argv)
> >>
> >>   		/* long options */
> >>   		case 0:
> >> -			print_usage(prgname);
> >> -			return -1;
> >> +			if (!strncmp(lgopts[option_index].name,
> >> +				CMD_LINE_OPT_IPV4_FRAG_SIZE,
> >> +				sizeof(CMD_LINE_OPT_IPV4_FRAG_SIZE))) {
> >> +				ret = parse_frag_size(optarg,
> >> +						MIN_IPV4_FRAG_SIZE,
> >> +						MAX_IPV4_FRAG_SIZE,
> >> +						sizeof(struct ipv4_hdr),
> >> +						&frag_size_v4);
> >> +				if (ret) {
> >> +					printf("invalid value: \"%s\" for "
> >> +						"parameter %s\n",
> >> +						optarg,
> >> +						lgopts[option_index].name);
> >> +					print_usage(prgname);
> >> +					return ret;
> >> +				}
> >> +			}
> >> +			if (!strncmp(lgopts[option_index].name,
> >> +				CMD_LINE_OPT_IPV6_FRAG_SIZE,
> >> +				sizeof(CMD_LINE_OPT_IPV6_FRAG_SIZE))) {
> >> +				ret = parse_frag_size(optarg,
> >> +						MIN_IPV6_FRAG_SIZE,
> >> +						MAX_IPV6_FRAG_SIZE,
> >> +						sizeof(struct ipv6_hdr),
> >> +						&frag_size_v6);
> >> +				if (ret) {
> >> +					printf("invalid value: \"%s\" for "
> >> +						"parameter %s\n",
> >> +						optarg,
> >> +						lgopts[option_index].name);
> >> +					print_usage(prgname);
> >> +					return ret;
> >> +				}
> >> +			}
> >> +
> >> +			break;
> >
> > Seems you ignored my comments for v1?
> > Konstantin
> Sorry, I forgot to reply to this comment.
> 'break' here is needed to avoid fall through to the default case.
> If any other long option is passed in command line then application
> should exit.

Please read my previous commnets:
http://dpdk.org/dev/patchwork/patch/23775/
and either address them in a proper manner, or explain why do you think
they are inappropriate.
Till then - NACK.
Konstantin


> Ashish
> >
> >>
> >>   		default:
> >>   			print_usage(prgname);
> >> --
> >> 2.7.4
> >
> >
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-30 12:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20  7:18 [PATCH] examples/ip_fragmentation: add fragmentation size support Ashish Jain
2017-05-24  7:28 ` Ashish Jain
2017-06-04 15:44 ` Ananyev, Konstantin
2017-06-14  9:19   ` Jain Ashish-B46179
2017-06-14 10:10     ` Ananyev, Konstantin
2017-06-30 12:18 ` [PATCH v2] example/ip_fragmentation: " Ashish Jain
2017-06-30 12:34   ` Ananyev, Konstantin
2017-06-30 12:50     ` Jain Ashish-B46179
2017-06-30 12:56       ` Ananyev, Konstantin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.