From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f196.google.com (mail-wr0-f196.google.com [209.85.128.196]) by dpdk.org (Postfix) with ESMTP id A88CB1B20D for ; Wed, 3 Jan 2018 18:29:56 +0100 (CET) Received: by mail-wr0-f196.google.com with SMTP id w68so2316284wrc.10 for ; Wed, 03 Jan 2018 09:29:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=SCC8reawzcSnU2uBqnPRWGMG2/w5lZVQOsS5PBuW54E=; b=CikydgFPtMuoaww++VgDwlp+8c+HD/6cvbs38Fvs2b0HChJfHM6uk0gquqt5Lq0SAP G1yvUCFOH699H/nmCjL0qZY19i9aOi8FdSG8BFSi33D20LNaU68MOcFGhSnw4Q2lSmr9 hDQCOpGhsloNbd67c5akpav8/SZ2a+YACQYVB4LeKUOO6cOjTUznzn4a3nABuV+AMuMO aUBruXZQFKM/4LsrxD0eBn7XY4aONz6ou5ho+b9La/GSZmi2ZOB9y5QviGZvitmEqkQ1 bsF9/7RK3J0POyYdAd3VzqL0jWPZ2dKpNqQI2SmOwUPj1OxYfdTmq3ihIt/USaggN9QK HLiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=SCC8reawzcSnU2uBqnPRWGMG2/w5lZVQOsS5PBuW54E=; b=qhEi0rk3rggfEp6sYICg9h85v/WqrOIm88iUKbR/bDtX0Z7wpqm+fnsYx0ayz4efs9 j8MvaII0ouRQVIzchf1kPPK59GnToaSTX6E62SS/j+56rAiJtI8lkhm5NxXBgTteY2+O F+6SxgQ/6MEB6FRVR4+vVhXe7kfgXhw5P0B+mKe4oVGbdMJziYnXfpi6Rr8fszjx3zZU 1+rC57+m48lc9kL2Re2iYx2DHAJXjtFiNwSb03By3L2SYze2mEjc3FVvJ8ltjUWO63P+ r9f/3MjK6du4bgiOWLbsbxxRqxao1DEznsmO6IapQ4gFKJEq3BxBkx4pr0L3OIgjboUV RUmQ== X-Gm-Message-State: AKGB3mK+XJCpVRU4LKXW8VgVUdIYHR+q4Q8gPEFrXuCTKZclTEUcdBWF KKCFjK+7fdtqbXH9wMfki2xEkA== X-Google-Smtp-Source: ACJfBouu7Y5qMiiuc/pXvjtC7FHnCqLsNF6YFavaL21F57g1D3u0o4NtGXMhm6vSYNvv7BOU2gfA5Q== X-Received: by 10.223.139.94 with SMTP id v30mr2144094wra.166.1515000596281; Wed, 03 Jan 2018 09:29:56 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 11sm1918435wrx.73.2018.01.03.09.29.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Jan 2018 09:29:55 -0800 (PST) Date: Wed, 3 Jan 2018 18:29:44 +0100 From: Adrien Mazarguil To: Shahaf Shuler Cc: nelio.laranjeiro@6wind.com, yskoh@mellanox.com, dev@dpdk.org Message-ID: <20180103172944.GF4256@6wind.com> References: <20171123120252.143695-1-shahafs@mellanox.com> <4e34d9ab1e5717d86a6c31a29ac2f6625209bc0d.1514963302.git.shahafs@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4e34d9ab1e5717d86a6c31a29ac2f6625209bc0d.1514963302.git.shahafs@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v2 7/7] net/mlx4: convert to new Rx offloads API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Jan 2018 17:29:56 -0000 In short, same comments as the TX patch, more below. On Wed, Jan 03, 2018 at 09:16:17AM +0200, Shahaf Shuler wrote: > Ethdev Rx offloads API has changed since: > > commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > > This commit support the new Rx offloads API. > > Signed-off-by: Shahaf Shuler > --- > drivers/net/mlx4/mlx4_ethdev.c | 10 ++--- > drivers/net/mlx4/mlx4_flow.c | 5 ++- > drivers/net/mlx4/mlx4_rxq.c | 78 ++++++++++++++++++++++++++++++++++--- > drivers/net/mlx4/mlx4_rxtx.h | 2 + > 4 files changed, 82 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c > index 63e00b1da..39a23ee7b 100644 > --- a/drivers/net/mlx4/mlx4_ethdev.c > +++ b/drivers/net/mlx4/mlx4_ethdev.c > @@ -766,13 +766,11 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info) > info->max_rx_queues = max; > info->max_tx_queues = max; > info->max_mac_addrs = RTE_DIM(priv->mac); > - info->rx_offload_capa = 0; > info->tx_offload_capa = mlx4_priv_get_tx_port_offloads(priv); > - if (priv->hw_csum) { > - info->rx_offload_capa |= (DEV_RX_OFFLOAD_IPV4_CKSUM | > - DEV_RX_OFFLOAD_UDP_CKSUM | > - DEV_RX_OFFLOAD_TCP_CKSUM); > - } > + info->rx_queue_offload_capa = > + mlx4_priv_get_rx_queue_offloads(priv); > + info->rx_offload_capa = (mlx4_priv_get_rx_port_offloads(priv) | > + info->rx_queue_offload_capa); > if (mlx4_get_ifname(priv, &ifname) == 0) > info->if_index = if_nametoindex(ifname); > info->hash_key_size = MLX4_RSS_HASH_KEY_SIZE; > diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c > index 69025da42..96a6a6fa7 100644 > --- a/drivers/net/mlx4/mlx4_flow.c > +++ b/drivers/net/mlx4/mlx4_flow.c > @@ -1232,7 +1232,7 @@ mlx4_flow_internal_next_vlan(struct priv *priv, uint16_t vlan) > * - MAC flow rules are generated from @p dev->data->mac_addrs > * (@p priv->mac array). > * - An additional flow rule for Ethernet broadcasts is also generated. > - * - All these are per-VLAN if @p dev->data->dev_conf.rxmode.hw_vlan_filter > + * - All these are per-VLAN if @p DEV_RX_OFFLOAD_VLAN_FILTER > * is enabled and VLAN filters are configured. > * > * @param priv > @@ -1300,7 +1300,8 @@ mlx4_flow_internal(struct priv *priv, struct rte_flow_error *error) > }; > struct ether_addr *rule_mac = ð_spec.dst; > rte_be16_t *rule_vlan = > - priv->dev->data->dev_conf.rxmode.hw_vlan_filter && > + (priv->dev->data->dev_conf.rxmode.offloads & > + DEV_RX_OFFLOAD_VLAN_FILTER) && > !priv->dev->data->promiscuous ? > &vlan_spec.tci : > NULL; > diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c > index 53313c56f..0cad28269 100644 > --- a/drivers/net/mlx4/mlx4_rxq.c > +++ b/drivers/net/mlx4/mlx4_rxq.c > @@ -663,6 +663,64 @@ mlx4_rxq_detach(struct rxq *rxq) > } > > /** > + * Returns the per-queue supported offloads. > + * > + * @param priv > + * Pointer to private structure. > + * > + * @return > + * Supported Tx offloads. > + */ > +uint64_t > +mlx4_priv_get_rx_queue_offloads(struct priv *priv) You should drop "priv", e.g.: mlx4_get_rx_queue_offloads() > +{ > + uint64_t offloads = DEV_RX_OFFLOAD_SCATTER; > + > + if (priv->hw_csum) > + offloads |= DEV_RX_OFFLOAD_CHECKSUM; > + return offloads; > +} > + > +/** > + * Returns the per-port supported offloads. > + * > + * @param priv > + * Pointer to private strucute. strucute -> structure > + * > + * @return > + * Supported Rx offloads. > + */ > +uint64_t > +mlx4_priv_get_rx_port_offloads(struct priv *priv __rte_unused) Same comment about "priv". Using (void) instead of __rte_unused for consistency with the rest of the PMD code is fine by the way. A subsequent patch can convert them all at once to __rte_unused if needed. > +{ > + uint64_t offloads = DEV_RX_OFFLOAD_VLAN_FILTER; > + > + return offloads; > +} > + > +/** > + * Checks if the per-queue offload configuration is valid. > + * > + * @param priv > + * Pointer to private structure. > + * @param offloads > + * Per-queue offloads configuration. > + * > + * @return > + * 1 if the configuration is valid, 0 otherwise. Better described as "Nonzero when configuration is valid." > + */ > +static int > +priv_is_rx_queue_offloads_allowed(struct priv *priv, uint64_t offloads) Ditto, with the same suggestion as for TX: [mlx4_]check_rx_queue_offloads() > +{ > + uint64_t port_offloads = priv->dev->data->dev_conf.rxmode.offloads; > + uint64_t port_supp_offloads = mlx4_priv_get_rx_port_offloads(priv); > + > + if (((port_offloads ^ offloads) & port_supp_offloads)) > + return 0; > + return 1; > +} Same comment and questions regarding this condition as for the TX patch (mandatory/requested/supported). > + > +/** > * DPDK callback to configure a Rx queue. > * > * @param dev > @@ -707,6 +765,16 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, > (void)conf; /* Thresholds configuration (ignored). */ > DEBUG("%p: configuring queue %u for %u descriptors", > (void *)dev, idx, desc); > + if (!priv_is_rx_queue_offloads_allowed(priv, conf->offloads)) { > + rte_errno = ENOTSUP; > + ERROR("%p: Rx queue offloads 0x%lx don't match port " > + "offloads 0x%lx or supported offloads 0x%lx", > + (void *)dev, conf->offloads, > + dev->data->dev_conf.rxmode.offloads, > + (mlx4_priv_get_rx_port_offloads(priv) | > + mlx4_priv_get_rx_queue_offloads(priv))); Should use "%" PRIx64 instead of "%lx". > + return -rte_errno; > + } > if (idx >= dev->data->nb_rx_queues) { > rte_errno = EOVERFLOW; > ERROR("%p: queue index out of range (%u >= %u)", > @@ -746,10 +814,10 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, > .elts_n = rte_log2_u32(desc), > .elts = elts, > /* Toggle Rx checksum offload if hardware supports it. */ > - .csum = (priv->hw_csum && > - dev->data->dev_conf.rxmode.hw_ip_checksum), > - .csum_l2tun = (priv->hw_csum_l2tun && > - dev->data->dev_conf.rxmode.hw_ip_checksum), > + .csum = priv->hw_csum && > + (conf->offloads & DEV_RX_OFFLOAD_CHECKSUM), > + .csum_l2tun = priv->hw_csum_l2tun && > + (conf->offloads & DEV_RX_OFFLOAD_CHECKSUM), > .l2tun_offload = priv->hw_csum_l2tun, > .stats = { > .idx = idx, > @@ -761,7 +829,7 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, > if (dev->data->dev_conf.rxmode.max_rx_pkt_len <= > (mb_len - RTE_PKTMBUF_HEADROOM)) { > ; > - } else if (dev->data->dev_conf.rxmode.enable_scatter) { > + } else if (conf->offloads & DEV_RX_OFFLOAD_SCATTER) { > uint32_t size = > RTE_PKTMBUF_HEADROOM + > dev->data->dev_conf.rxmode.max_rx_pkt_len; > diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h > index 91971c4fb..bcb76ee27 100644 > --- a/drivers/net/mlx4/mlx4_rxtx.h > +++ b/drivers/net/mlx4/mlx4_rxtx.h > @@ -166,6 +166,8 @@ int mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, > const struct rte_eth_rxconf *conf, > struct rte_mempool *mp); > void mlx4_rx_queue_release(void *dpdk_rxq); > +uint64_t mlx4_priv_get_rx_port_offloads(struct priv *priv); > +uint64_t mlx4_priv_get_rx_queue_offloads(struct priv *priv); Declarations should come in the same order as in mlx4_rxq.c. > > /* mlx4_rxtx.c */ > > -- > 2.12.0 > -- Adrien Mazarguil 6WIND