From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 4D187A0A03;
	Mon, 18 Jan 2021 20:02:12 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id DF063140DE7;
	Mon, 18 Jan 2021 20:02:11 +0100 (CET)
Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com
 [209.85.219.45])
 by mails.dpdk.org (Postfix) with ESMTP id D869B140DE3
 for <dev@dpdk.org>; Mon, 18 Jan 2021 20:02:09 +0100 (CET)
Received: by mail-qv1-f45.google.com with SMTP id l7so8001302qvt.4
 for <dev@dpdk.org>; Mon, 18 Jan 2021 11:02:09 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc; bh=oV3DU9UaVjXlQKmoP5A8GgALlULsD8VAq19uI3VCIyQ=;
 b=VtSZAwZJMB8J72dqR8rT3WHa/w/sUyDB5xSUKaJo9ixOqqzoVHyEikCVgmmgZI41s8
 QqEsRDNr+C998lbigQE0aJXBAIksDjd/nx+Bx6Wd5O5l5Wo29/DnIh8I33kKrsorNeef
 lhLEb7IJWk0x7Rte+kADttKCkOyyRnmFlSkOE=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=oV3DU9UaVjXlQKmoP5A8GgALlULsD8VAq19uI3VCIyQ=;
 b=SHTAGFTr1ogySNc4HVtMHlUelPWuPKjcEU0gBS481dPmMJysspGLR0i6ZNHWRif722
 YGw+N8JjyzQ1/ZsfQ0GyuSomKldQfcb07xFq7nsFE4NPOEq5t9kS9lDYmn9H/cMFUR+J
 NXdJ1c9zmqQVUvWWL6u2Q0Y1cfjyAFqoEPW1HnrgRgMG6DBIjm4KMhoXrZOvHqN4sLWV
 fRZHHRwxEr8J0OopYEDMe/3IQnS0Vl/FdC58yfOyIQj22yMVDj8Q8Hla4+OJ1GfwSH83
 ygaB4JNUl8jHCilYL86z/VSSWcrWexe4+17pYplv6+ubcZvfUAcTNemieF/GmNvxqPd+
 wJSg==
X-Gm-Message-State: AOAM532ieSrT9MIkRIc766hFJmMsxNf0RN/vVVYf1XJKriW5nrptMcYI
 kist+uHcye35p9Qzvk/6oM9IOxoRkUSNMsAWepNTDA==
X-Google-Smtp-Source: ABdhPJwtc2GrDiGM1LzfTtXrY/cdv0TG+llAzlD98iD+4FOTr53C9Ek6JMmlECTCw+cFqd9ZxH0cWTKU7L05VtPGdhU=
X-Received: by 2002:a0c:a994:: with SMTP id a20mr1175368qvb.30.1610996528911; 
 Mon, 18 Jan 2021 11:02:08 -0800 (PST)
MIME-Version: 1.0
References: <1608303356-13089-2-git-send-email-xuemingl@nvidia.com>
 <1610968623-31345-1-git-send-email-xuemingl@nvidia.com>
 <1610968623-31345-8-git-send-email-xuemingl@nvidia.com>
In-Reply-To: <1610968623-31345-8-git-send-email-xuemingl@nvidia.com>
From: Ajit Khaparde <ajit.khaparde@broadcom.com>
Date: Mon, 18 Jan 2021 11:01:52 -0800
Message-ID: <CACZ4nhuJvYv384bf2+YW3cjdW8bDcN9B1=7teYdzf_0uy-tOng@mail.gmail.com>
To: Xueming Li <xuemingl@nvidia.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
 Ferruh Yigit <ferruh.yigit@intel.com>, 
 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
 Olivier Matz <olivier.matz@6wind.com>, 
 dpdk-dev <dev@dpdk.org>, Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
 Asaf Penso <asafp@nvidia.com>
Content-Type: multipart/signed; protocol="application/pkcs7-signature";
 micalg=sha-256; boundary="000000000000e0092d05b9315b78"
X-Content-Filtered-By: Mailman/MimeDel 2.1.29
Subject: Re: [dpdk-dev] [PATCH v4 7/9] devarg: change representor ID to
 bitmap
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

--000000000000e0092d05b9315b78
Content-Type: text/plain; charset="UTF-8"

On Mon, Jan 18, 2021 at 3:18 AM Xueming Li <xuemingl@nvidia.com> wrote:
>
> The NIC can have multiple PCIe links and can be attached to multiple
> hosts, for example the same single NIC can be shared for multiple server
> units in the rack. On each PCIe link NIC can provide multiple PFs and
> VFs/SFs based on these ones. The full representor identifier consists of
> three indices - controller index, PF index, and VF or SF index (if any).
>
> SR-IOV and SubFunction are created on top of PF. PF index is introduced
> because there might be multiple PFs in the bonding configuration and
> only bonding device is probed.
>
> In eth representor comparator callback, ethdev was compared with devarg.
> Since ethdev representor port didn't contain controller index and PF
> index information, callback returned representor from other PF or
> controller.
>
> This patch changes representor ID to bitmap so that the ethdev
> representor comparer callback returns correct ethdev by comparing full
> representor information including: controller index, PF index,
> representor type, SF or VF index.
>
> Representor ID bitmap definition:
>  xxxx xxxx xxxx xxxx
>  |||| |LLL LLLL LLLL vf/sf id
>  |||| L 1:sf, 0:vf
>  ||LL pf id
>  LL controller(host) id
>
> This approach keeps binary compatibility with all drivers, VF
> representor id matches with simple id for non-bonding and non-multi-host
> configurations.
>
> In the future, the representor ID field and each section should extend
> to bigger width to support more devices.
>
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Not just in this patch, there is a lot of info in the commit logs.
Unless I missed it, I don't see an update to
doc/guides/prog_guide/switch_representation.rst
Can you please update that.

> ---
>  drivers/net/bnxt/bnxt_reps.c             |  3 +-
>  drivers/net/enic/enic_vf_representor.c   |  3 +-
>  drivers/net/i40e/i40e_vf_representor.c   |  3 +-
>  drivers/net/ixgbe/ixgbe_vf_representor.c |  3 +-
>  drivers/net/mlx5/linux/mlx5_os.c         |  4 +-
>  lib/librte_ethdev/ethdev_private.c       |  5 ++-
>  lib/librte_ethdev/rte_class_eth.c        | 38 +++++++++++++----
>  lib/librte_ethdev/rte_ethdev.c           | 26 ++++++++++++
>  lib/librte_ethdev/rte_ethdev_driver.h    | 53 ++++++++++++++++++++++++
>  lib/librte_ethdev/version.map            |  2 +
>  10 files changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_reps.c b/drivers/net/bnxt/bnxt_reps.c
> index f7bbf77d3f..34febc2d1e 100644
> --- a/drivers/net/bnxt/bnxt_reps.c
> +++ b/drivers/net/bnxt/bnxt_reps.c
> @@ -186,7 +186,8 @@ int bnxt_representor_init(struct rte_eth_dev *eth_dev, void *params)
>
>         eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> -       eth_dev->data->representor_id = rep_params->vf_id;
> +       eth_dev->data->representor_id = rte_eth_representor_id_encode(
> +               0, 0, RTE_ETH_REPRESENTOR_VF, rep_params->vf_id);
>
>         rte_eth_random_addr(vf_rep_bp->dflt_mac_addr);
>         memcpy(vf_rep_bp->mac_addr, vf_rep_bp->dflt_mac_addr,
> diff --git a/drivers/net/enic/enic_vf_representor.c b/drivers/net/enic/enic_vf_representor.c
> index c2c03c0281..632317af15 100644
> --- a/drivers/net/enic/enic_vf_representor.c
> +++ b/drivers/net/enic/enic_vf_representor.c
> @@ -674,7 +674,8 @@ int enic_vf_representor_init(struct rte_eth_dev *eth_dev, void *init_params)
>         eth_dev->dev_ops = &enic_vf_representor_dev_ops;
>         eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> -       eth_dev->data->representor_id = vf->vf_id;
> +       eth_dev->data->representor_id = rte_eth_representor_id_encode(
> +               0, 0, RTE_ETH_REPRESENTOR_VF, vf->vf_id);
>         eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr_vf",
>                 sizeof(struct rte_ether_addr) *
>                 ENIC_UNICAST_PERFECT_FILTERS, 0);
> diff --git a/drivers/net/i40e/i40e_vf_representor.c b/drivers/net/i40e/i40e_vf_representor.c
> index 9e40406a3d..d90d0fdb9d 100644
> --- a/drivers/net/i40e/i40e_vf_representor.c
> +++ b/drivers/net/i40e/i40e_vf_representor.c
> @@ -510,7 +510,8 @@ i40e_vf_representor_init(struct rte_eth_dev *ethdev, void *init_params)
>
>         ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> -       ethdev->data->representor_id = representor->vf_id;
> +       ethdev->data->representor_id = rte_eth_representor_id_encode(
> +                       0, 0, RTE_ETH_REPRESENTOR_VF, representor->vf_id);
>
>         /* Setting the number queues allocated to the VF */
>         ethdev->data->nb_rx_queues = vf->vsi->nb_qps;
> diff --git a/drivers/net/ixgbe/ixgbe_vf_representor.c b/drivers/net/ixgbe/ixgbe_vf_representor.c
> index 8185f0d3bb..e15b794761 100644
> --- a/drivers/net/ixgbe/ixgbe_vf_representor.c
> +++ b/drivers/net/ixgbe/ixgbe_vf_representor.c
> @@ -196,7 +196,8 @@ ixgbe_vf_representor_init(struct rte_eth_dev *ethdev, void *init_params)
>                 return -ENODEV;
>
>         ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
> -       ethdev->data->representor_id = representor->vf_id;
> +       ethdev->data->representor_id = rte_eth_representor_id_encode(
> +                       0, 0, RTE_ETH_REPRESENTOR_VF, representor->vf_id);
>
>         /* Set representor device ops */
>         ethdev->dev_ops = &ixgbe_vf_representor_dev_ops;
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
> index caead107b0..4d7940bcca 100644
> --- a/drivers/net/mlx5/linux/mlx5_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> @@ -1025,7 +1025,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  #endif
>         /* representor_id field keeps the unmodified VF index. */
>         priv->representor_id = switch_info->representor ?
> -                              switch_info->port_name : -1;
> +               rte_eth_representor_id_encode(0, 0, RTE_ETH_REPRESENTOR_VF,
> +                                             switch_info->port_name) :
> +               -1;
>         /*
>          * Look for sibling devices in order to reuse their switch domain
>          * if any, otherwise allocate one.
> diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c
> index 57473b5a39..0b3b4aa871 100644
> --- a/lib/librte_ethdev/ethdev_private.c
> +++ b/lib/librte_ethdev/ethdev_private.c
> @@ -106,10 +106,13 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
>  }
>
>  /*
> - * representor format:
> + * Parse representor ports, expand and update representor port ID.
> + * Representor format:
>   *   #: range or single number of VF representor - legacy
>   *   [[c#]pf#]vf#: VF port representor/s
>   *   [[c#]pf#]sf#: SF port representor/s
> + *
> + * See RTE_ETH_REPR() for representor ID format.
>   */
>  int
>  rte_eth_devargs_parse_representor_ports(char *str, void *data)
> diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
> index efe6149df5..994db96960 100644
> --- a/lib/librte_ethdev/rte_class_eth.c
> +++ b/lib/librte_ethdev/rte_class_eth.c
> @@ -66,8 +66,8 @@ eth_representor_cmp(const char *key __rte_unused,
>         int ret;
>         char *values;
>         const struct rte_eth_dev_data *data = opaque;
> -       struct rte_eth_devargs representors;
> -       uint16_t index;
> +       struct rte_eth_devargs eth_da;
> +       uint16_t index, c, p, f;
>
>         if ((data->dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0)
>                 return -1; /* not a representor port */
> @@ -76,17 +76,39 @@ eth_representor_cmp(const char *key __rte_unused,
>         values = strdup(value);
>         if (values == NULL)
>                 return -1;
> -       memset(&representors, 0, sizeof(representors));
> -       ret = rte_eth_devargs_parse_representor_ports(values, &representors);
> +       memset(&eth_da, 0, sizeof(eth_da));
> +       ret = rte_eth_devargs_parse_representor_ports(values, &eth_da);
>         free(values);
>         if (ret != 0)
>                 return -1; /* invalid devargs value */
>
> +       /* Set default values. */
> +       if (eth_da.nb_mh_controllers == 0) {
> +               eth_da.nb_mh_controllers = 1;
> +               eth_da.mh_controllers[0] = 0;
> +       }
> +       if (eth_da.nb_ports == 0) {
> +               eth_da.nb_ports = 1;
> +               eth_da.ports[0] = 0;
> +       }
> +       if (eth_da.nb_representor_ports == 0) {
> +               eth_da.nb_representor_ports = 1;
> +               eth_da.representor_ports[0] = 0;
> +       }
>         /* Return 0 if representor id is matching one of the values. */
> -       for (index = 0; index < representors.nb_representor_ports; index++)
> -               if (data->representor_id ==
> -                               representors.representor_ports[index])
> -                       return 0;
> +       for (c = 0; c < eth_da.nb_mh_controllers; ++c) {
> +               for (p = 0; p < eth_da.nb_ports; ++p) {
> +                       for (f = 0; f < eth_da.nb_representor_ports; ++f) {
> +                               index = rte_eth_representor_id_encode(
> +                                       eth_da.mh_controllers[c],
> +                                       eth_da.ports[p],
> +                                       eth_da.type,
> +                                       eth_da.representor_ports[f]);
> +                               if (data->representor_id == index)
> +                                       return 0;
> +                       }
> +               }
> +       }
>         return -1; /* no match */
>  }
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 2ac51ac149..276f42e54b 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -5556,6 +5556,32 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
>         return result;
>  }
>
> +uint16_t
> +rte_eth_representor_id_encode(uint16_t controller, uint16_t pf,
> +                             enum rte_eth_representor_type type,
> +                             uint16_t representor_port)
> +{
> +       return (((controller & 3) << 14) |
> +               ((pf & 3) << 12) |
> +               (!!(type == RTE_ETH_REPRESENTOR_SF) << 11) |
> +               (representor_port & 0x7ff));
> +}
> +
> +uint16_t
> +rte_eth_representor_id_parse(const uint16_t representor_id,
> +                            uint16_t *controller, uint16_t *pf,
> +                            enum rte_eth_representor_type *type)
> +{
> +       if (controller)
> +               *controller = (representor_id >> 14) & 3;
> +       if (pf)
> +               *pf = (representor_id >> 12) & 3;
> +       if (type)
> +               *type = ((representor_id >> 11) & 1) ?
> +                       RTE_ETH_REPRESENTOR_SF : RTE_ETH_REPRESENTOR_VF;
> +       return representor_id & 0x7ff;
> +}
> +
>  static int
>  eth_dev_handle_port_list(const char *cmd __rte_unused,
>                 const char *params __rte_unused,
> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index 8e04634660..0d8893693e 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -1218,6 +1218,59 @@ struct rte_eth_devargs {
>         enum rte_eth_representor_type type; /* type of representor */
>  };
>
> +#define RTE_NO_REPRESENTOR_ID UINT16_MAX /**< No representor ID. */
> +
> +/**
> + * PMD helper function to encode representor ID
> + *
> + * The compact format is used for device iterator that comparing
> + * ethdev representor ID with target devargs.
> + *
> + * xxxx xxxx xxxx xxxx
> + * |||| |LLL LLLL LLLL vf/sf id
> + * |||| L 1:sf, 0:vf
> + * ||LL pf id
> + * LL controller(host) id
> + *
> + * @param controller
> + *  Controller ID.
> + * @param pf
> + *  PF port ID.
> + * @param type
> + *  Representor type.
> + * @param representor_port
> + *  Representor port ID.
> + *
> + * @return
> + *   Encoded representor ID.
> + */
> +__rte_internal
> +uint16_t
> +rte_eth_representor_id_encode(uint16_t controller, uint16_t pf,
> +                             enum rte_eth_representor_type type,
> +                             uint16_t representor_port);
> +
> +/**
> + * PMD helper function to parse representor ID
> + *
> + * @param representor_id
> + *  Representor ID.
> + * @param controller
> + *  Parsed controller ID.
> + * @param pf
> + *  Parsed PF port ID.
> + * @param type
> + *  Parsed representor type.
> + *
> + * @return
> + *   Parsed representor port ID.
> + */
> +__rte_internal
> +uint16_t
> +rte_eth_representor_id_parse(const uint16_t representor_id,
> +                            uint16_t *controller, uint16_t *pf,
> +                            enum rte_eth_representor_type *type);
> +
>  /**
>   * PMD helper function to parse ethdev arguments
>   *
> diff --git a/lib/librte_ethdev/version.map b/lib/librte_ethdev/version.map
> index d3f5410806..44edaed507 100644
> --- a/lib/librte_ethdev/version.map
> +++ b/lib/librte_ethdev/version.map
> @@ -257,6 +257,8 @@ INTERNAL {
>         rte_eth_dev_release_port;
>         rte_eth_dev_internal_reset;
>         rte_eth_devargs_parse;
> +       rte_eth_representor_id_encode;
> +       rte_eth_representor_id_parse;
>         rte_eth_dma_zone_free;
>         rte_eth_dma_zone_reserve;
>         rte_eth_hairpin_queue_peer_bind;
> --
> 2.25.1
>

--000000000000e0092d05b9315b78--