> -----Original Message-----
> From: Jain Ashish-B46179 [mailto:ashish.j...@nxp.com]
> Sent: Wednesday, June 14, 2017 10:20 AM
> To: Ananyev, Konstantin <konstantin.anan...@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.j...@nxp.com]
> >> Sent: Thursday, April 20, 2017 8:18 AM
> >> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.anan...@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.j...@nxp.com>
> >> Signed-off-by: Hemant Agrawal <hemant.agra...@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
> >
> >
>
