Re: [PATCH v4 2/3] handle pps limit parameter

[Wei Liu <wei.liu2@citrix.com>]

Suggest adding "libxl: " prefix in subject line.

Plus, you should CC Ian Jackson for toolstack patches (which I've
already done for this mail).

On Mon, Aug 05, 2013 at 05:13:09PM +0200, William Dauchy wrote:
> adapt libxl to handle pps limit parameter
> the new pps limit can be defined using a '&' symbol after
> the rate limit for example:
> YYMb/s&XXKpps@ZZms
> or
> YYMb/s@ZZms&XXKpps
> or
> YYMb/s&XXKpps in such case default 50ms interval will be used
> 

One question I need to ask is that is it possible for user to only
specify PPS? From previous email and the code below it doesn't seem to
allow users to do so. I know rate and PPS use the same interval but that
doesn't mean that the later depend on the former, right?


Wei.


> Signed-off-by: Ahmed Amamou <ahmed@gandi.net>
> Signed-off-by: William Dauchy <william@gandi.net>
> Signed-off-by: Kamel Haddadou <kamel@gandi.net>
> ---
>  tools/libxl/libxl.c         |    3 ++
>  tools/libxl/libxl_types.idl |    1 +
>  tools/libxl/libxlu_vif.c    |   70 +++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 71 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 3236aa9..11572bc 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2920,6 +2920,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
>          flexarray_append(back, libxl__sprintf(gc, "%"PRIu64",%"PRIu32"",
>                              nic->rate_bytes_per_interval,
>                              nic->rate_interval_usecs));
> +        flexarray_append(back, "pps");
> +        flexarray_append(back, libxl__sprintf(gc, "%"PRIu64"",
> +                            nic->rate_packets_per_interval));
>      }
>  
>      flexarray_append(back, "bridge");
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index d218a2d..be2ae94 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -390,6 +390,7 @@ libxl_device_nic = Struct("device_nic", [
>      ("script", string),
>      ("nictype", libxl_nic_type),
>      ("rate_bytes_per_interval", uint64),
> +    ("rate_packets_per_interval", uint64),
>      ("rate_interval_usecs", uint32),
>      ("gatewaydev", string),
>      ])
> diff --git a/tools/libxl/libxlu_vif.c b/tools/libxl/libxlu_vif.c
> index 3b3de0f..3d491d8 100644
> --- a/tools/libxl/libxlu_vif.c
> +++ b/tools/libxl/libxlu_vif.c
> @@ -3,6 +3,7 @@
>  
>  static const char *vif_bytes_per_sec_re = "^[0-9]+[GMK]?[Bb]/s$";
>  static const char *vif_internal_usec_re = "^[0-9]+[mu]?s?$";
> +static const char *vif_packets_per_sec_re = "^[0-9]+[GMK]?pps$";
>  
>  static void xlu__vif_err(XLU_Config *cfg, const char *msg, const char *rate) {
>      fprintf(cfg->report,
> @@ -49,6 +50,43 @@ out:
>      return rc;
>  }
>  
> +static int vif_parse_rate_packets_per_sec(XLU_Config *cfg, const char *packet,
> +                                        uint64_t *packets_per_sec)
> +{
> +    regex_t rec;
> +    uint64_t tmp = 0;
> +    const char *p;
> +    int rc = 0;
> +
> +    regcomp(&rec, vif_packets_per_sec_re, REG_EXTENDED|REG_NOSUB);
> +    if (regexec(&rec, packet, 0, NULL, 0)) {
> +        xlu__vif_err(cfg, "invalid pps", packet);
> +        rc = EINVAL;
> +        goto out;
> +    }
> +
> +    p = packet;
> +    tmp = strtoull(p, (char**)&p, 0);
> +    if (tmp == 0 || tmp > UINT32_MAX || errno == ERANGE) {
> +        xlu__vif_err(cfg, "pps overflow", packet);
> +        rc = EOVERFLOW;
> +        goto out;
> +    }
> +
> +    if (*p == 'G')
> +       tmp *= 1000 * 1000 * 1000;
> +    else if (*p == 'M')
> +       tmp *= 1000 * 1000;
> +    else if (*p == 'K')
> +       tmp *= 1000;
> +
> +    *packets_per_sec = tmp;
> +
> +out:
> +    regfree(&rec);
> +    return rc;
> +}
> +
>  static int vif_parse_rate_interval_usecs(XLU_Config *cfg, const char *interval,
>                                           uint32_t *interval_usecs)
>  {
> @@ -94,22 +132,35 @@ int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate, libxl_device_nic *nic)
>  {
>      uint64_t bytes_per_sec = 0;
>      uint64_t bytes_per_interval = 0;
> +    uint64_t packets_per_sec = 0;
> +    uint64_t packets_per_interval = 0;
>      uint32_t interval_usecs = 50000UL; /* Default to 50ms */
> -    char *ratetok, *tmprate;
> +    char *ratetok, *tmprate, *tmp_pps, *tmpint;
>      int rc = 0;
>  
> +    /* rate string need to be duplicated because strtok may change it */
>      tmprate = strdup(rate);
> +    tmp_pps = strdup(rate);
> +    tmpint = strdup(rate);
> +
>      if (!strcmp(tmprate,"")) {
>          xlu__vif_err(cfg, "no rate specified", rate);
>          rc = EINVAL;
>          goto out;
>      }
>  
> -    ratetok = strtok(tmprate, "@");
> +    /* accepted rate string are as follow:
> +     * rate&pps@interval or rate@interval&pps
> +     */
> +
> +    /* ratetok contains the first token */
> +    ratetok = strtok(tmprate, "@&");
>      rc = vif_parse_rate_bytes_per_sec(cfg, ratetok, &bytes_per_sec);
>      if (rc) goto out;
>  
> -    ratetok = strtok(NULL, "@");
> +    ratetok = strtok(tmpint, "@");
> +    ratetok = strtok(NULL, "@&");
> +    /* ratetok contains the first token following the '@' */
>      if (ratetok != NULL) {
>          rc = vif_parse_rate_interval_usecs(cfg, ratetok, &interval_usecs);
>          if (rc) goto out;
> @@ -121,14 +172,27 @@ int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate, libxl_device_nic *nic)
>          goto out;
>      }
>  
> +    ratetok = strtok(tmp_pps, "&");
> +    ratetok = strtok(NULL, "&@");
> +    /* ratetok contains the first token following the '&' */
> +    if (ratetok != NULL) {
> +        rc = vif_parse_rate_packets_per_sec(cfg, ratetok, &packets_per_sec);
> +        if (rc) goto out;
> +    }
> +
>      bytes_per_interval =
>          (((uint64_t) bytes_per_sec * (uint64_t) interval_usecs) / 1000000UL);
> +    packets_per_interval =
> +        (((uint64_t) packets_per_sec * (uint64_t) interval_usecs) / 1000000UL);
>  
>      nic->rate_interval_usecs = interval_usecs;
>      nic->rate_bytes_per_interval = bytes_per_interval;
> +    nic->rate_packets_per_interval = packets_per_interval;
>  
>  out:
>      free(tmprate);
> +    free(tmp_pps);
> +    free(tmpint);
>      return rc;
>  }
>  
> -- 
> 1.7.9.5