Thanks for the change in log level

Acked-by: Vipin Varghese <vipin.vargh...@intel.com>

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, July 13, 2018 10:56 PM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haa...@intel.com>;
> tho...@monjalon.net; Varghese, Vipin <vipin.vargh...@intel.com>
> Subject: [PATCH v3] eal/service: improve error checking of coremasks
> 
> This commit improves the error checking performed on the core masks (or
> lists) of the service cores, in particular with respect to the data-plane 
> (RTE)
> cores of DPDK.
> 
> With this commit, invalid configurations are detected at runtime, and warning
> messages are printed to inform the user.
> 
> For example specifying the coremask as 0xf, and the service coremask as
> 0xff00 is invalid as not all service-cores are contained within the coremask. 
> A
> warning is now printed to inform the user.
> 
> Reported-by: Vipin Varghese <vipin.vargh...@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com>
> 
> ---
> 
> v3:
> - Use WARNING instead of ERR log level (Vipin)
> - Put strings on one line for grep-ability (Harry)
> 
> 
> v2, thanks for review:
> - Consistency in message endings - vs . (Thomas)
> - Wrap lines as they're very long otherwise (Thomas)
> 
> Cc: tho...@monjalon.net
> Cc: vipin.vargh...@intel.com
> 
> @Thomas, RC2 is also OK for this patch in my opinion
> ---
>  lib/librte_eal/common/eal_common_options.c | 39
> ++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 80f11d0..bee0f2d 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -321,6 +321,7 @@ eal_parse_service_coremask(const char *coremask)
>       unsigned int count = 0;
>       char c;
>       int val;
> +     uint32_t taken_lcore_count = 0;
> 
>       if (coremask == NULL)
>               return -1;
> @@ -364,6 +365,10 @@ eal_parse_service_coremask(const char *coremask)
>                                               "lcore %u unavailable\n",
> idx);
>                                       return -1;
>                               }
> +
> +                             if (cfg->lcore_role[idx] == ROLE_RTE)
> +                                     taken_lcore_count++;
> +
>                               lcore_config[idx].core_role = ROLE_SERVICE;
>                               count++;
>                       }
> @@ -380,11 +385,27 @@ eal_parse_service_coremask(const char
> *coremask)
>       if (count == 0)
>               return -1;
> 
> +     if (core_parsed && taken_lcore_count != count) {
> +             RTE_LOG(WARNING, EAL,
> +                     "Not all service cores are in the coremask. Please
> ensure -c or -l includes service cores\n");
> +     }
> +
>       cfg->service_lcore_count = count;
>       return 0;
>  }
> 
>  static int
> +eal_service_cores_parsed(void)
> +{
> +     int idx;
> +     for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> +             if (lcore_config[idx].core_role == ROLE_SERVICE)
> +                     return 1;
> +     }
> +     return 0;
> +}
> +
> +static int
>  eal_parse_coremask(const char *coremask)  {
>       struct rte_config *cfg = rte_eal_get_configuration(); @@ -393,6
> +414,10 @@ eal_parse_coremask(const char *coremask)
>       char c;
>       int val;
> 
> +     if (eal_service_cores_parsed())
> +             RTE_LOG(WARNING, EAL,
> +                     "Service cores parsed before dataplane cores Please
> ensure -c is
> +before -s or -S.\n");
> +
>       if (coremask == NULL)
>               return -1;
>       /* Remove all blank characters ahead and after .
> @@ -424,6 +449,7 @@ eal_parse_coremask(const char *coremask)
>                                               "unavailable\n", idx);
>                                       return -1;
>                               }
> +
>                               cfg->lcore_role[idx] = ROLE_RTE;
>                               lcore_config[idx].core_index = count;
>                               count++;
> @@ -455,6 +481,7 @@ eal_parse_service_corelist(const char *corelist)
>       unsigned count = 0;
>       char *end = NULL;
>       int min, max;
> +     uint32_t taken_lcore_count = 0;
> 
>       if (corelist == NULL)
>               return -1;
> @@ -496,6 +523,9 @@ eal_parse_service_corelist(const char *corelist)
>                                                       idx);
>                                               return -1;
>                                       }
> +                                     if (cfg->lcore_role[idx] == ROLE_RTE)
> +                                             taken_lcore_count++;
> +
>                                       lcore_config[idx].core_role =
>                                                       ROLE_SERVICE;
>                                       count++;
> @@ -510,6 +540,11 @@ eal_parse_service_corelist(const char *corelist)
>       if (count == 0)
>               return -1;
> 
> +     if (core_parsed && taken_lcore_count != count) {
> +             RTE_LOG(WARNING, EAL,
> +                     "not all service cores were in the coremask. Please
> ensure -c or -l includes service cores\n");
> +     }
> +
>       return 0;
>  }
> 
> @@ -522,6 +557,10 @@ eal_parse_corelist(const char *corelist)
>       char *end = NULL;
>       int min, max;
> 
> +     if (eal_service_cores_parsed())
> +             RTE_LOG(WARNING, EAL,
> +                     "Service cores parsed before dataplane cores. Please
> ensure -l is
> +before -s or -S.\n");
> +
>       if (corelist == NULL)
>               return -1;
> 
> --
> 2.7.4

Reply via email to